Let unaliased Args track which Alias they were created from, and use that in Arg::getAsString() for diagnostics

[thakis]

Summary

With this, clang-cl /source-charset:utf-16 test.cc now prints invalid value 'utf-16' in '/source-charset:utf-16' instead of invalid value 'utf-16' in '-finput-charset=utf-16' before, and several other clang-cl flags produce much less confusing output as well.

Fixes PR29106.

Since an arg and its alias can have different arg types (joined vs not) and different values (because of AliasArgs<>), I chose to give the Alias its own Arg object. For convenience, I just store the alias directly in the unaliased arg – there aren't many arg objects at runtime, so that seems ok.

Finally, I changed Arg::getAsString() to use the alias's representation if it's present – that function was already documented as being the suitable function for diagnostics, and most callers already used it for diagnostics.

Implementation-wise, Arg::accept() previously used to parse things as the unaliased option. The core of that switch is now extracted into a new function acceptInternal() which parses as the _aliased_ option, and the previously-intermingled unaliasing is now done as an explicit step afterwards.

(This also changes one place in lld that didn't use getAsString() for diagnostics, so that that one place now also prints the flag as the user wrote it, not as it looks after it went through unaliasing.)

[rnk]

lgtm

llvm/include/llvm/Option/Arg.h

59 ↗

(On Diff #208192)

I'm not sure this is really true:

For convenience, I just store the alias directly in the unaliased arg – there aren't many arg objects at runtime, so that seems ok.

Command lines can end up being quite long. Consider:

$ wc -lc out/clang/browser_tests.exe.rsp
  5381 361257 out/clang/browser_tests.exe.rsp

However, if someone comes along and cares about this, they can try shrinking this SmallVector instead, and using something similar to TinyPtrVector.

[thakis]

lgtm

Thanks!

5381 361257 out/clang/browser_tests.exe.rsp

Looking at that file, that looks like 10554 distinct args from the rsp file. That's ~80kB for the additional pointer, which for a link that's going to consume hundreds of MB isn't all that much overhead.

(If we did care about Arg size for some reason, there are a whole bunch of things we can do – e.g. removing CommaJoined and the OwnsValues member, or making the Option pointer in the arg and the unaliased arg not duplicate the OptTable pointer, etc.)

[hans]

I think this is going to be my favourite new feature in clang 9 :-) Do you want to put a small note in docs/ReleaseNotes.rst?

cfe/trunk/test/Driver/arm-execute-only.c

 // RUN: not %clang -c -target thumbv6m-eabi -mexecute-only %s 2>&1 | \
 // RUN:   FileCheck --check-prefix CHECK-EXECUTE-ONLY-NOT-SUPPORTED %s
+// CHECK-EXECUTE-ONLY-NOT-SUPPORTED: error: execute only is not supported for the thumbv6m sub-architecture
 
 // RUN: not %clang -target armv8m.main-eabi -mexecute-only -mno-movt %s 2>&1 \
 // RUN:    | FileCheck %s -check-prefix CHECK-EXECUTE-ONLY-NO-MOVT
+// CHECK-EXECUTE-ONLY-NO-MOVT: error: option '-mexecute-only' cannot be specified with '-mno-movt'
 
 // RUN: not %clang -target armv8m.main-eabi -mexecute-only -mlong-calls %s 2>&1 \
 // RUN:    | FileCheck %s -check-prefix CHECK-EXECUTE-ONLY-LONG-CALLS
+// CHECK-EXECUTE-ONLY-LONG-CALLS: error: option '-mexecute-only' cannot be specified with '-mlong-calls'
 
+// RUN: %clang -target armv7m-eabi -x assembler -mexecute-only %s -c -### 2>&1 \
+// RUN:    | FileCheck %s -check-prefix CHECK-NO-EXECUTE-ONLY -check-prefix CHECK-NO-EXECUTE-ONLY-ASM
+// CHECK-NO-EXECUTE-ONLY-ASM: warning: argument unused during compilation: '-mexecute-only'
 
 // -mpure-code flag for GCC compatibility
 // RUN: not %clang -c -target thumbv6m-eabi -mpure-code %s 2>&1 | \
 // RUN:   FileCheck --check-prefix CHECK-EXECUTE-ONLY-NOT-SUPPORTED %s
 
 // RUN: not %clang -target armv8m.main-eabi -mpure-code -mno-movt %s 2>&1 \
-// RUN:    | FileCheck %s -check-prefix CHECK-EXECUTE-ONLY-NO-MOVT
+// RUN:    | FileCheck %s -check-prefix CHECK-PURE-CODE-NO-MOVT
+// CHECK-PURE-CODE-NO-MOVT: error: option '-mpure-code' cannot be specified with '-mno-movt'
 
 // RUN: not %clang -target armv8m.main-eabi -mpure-code -mlong-calls %s 2>&1 \
-// RUN:    | FileCheck %s -check-prefix CHECK-EXECUTE-ONLY-LONG-CALLS
-
-// RUN: %clang -target armv7m-eabi -x assembler -mexecute-only %s -c -### 2>&1 \
-// RUN:    | FileCheck %s -check-prefix CHECK-NO-EXECUTE-ONLY -check-prefix CHECK-NO-EXECUTE-ONLY-ASM
-
-// CHECK-EXECUTE-ONLY-NOT-SUPPORTED: error: execute only is not supported for the thumbv6m sub-architecture
-// CHECK-EXECUTE-ONLY-NO-MOVT: error: option '-mexecute-only' cannot be specified with '-mno-movt'
-// CHECK-EXECUTE-ONLY-LONG-CALLS: error: option '-mexecute-only' cannot be specified with '-mlong-calls'
-// CHECK-NO-EXECUTE-ONLY-ASM: warning: argument unused during compilation: '-mexecute-only'
+// RUN:    | FileCheck %s -check-prefix CHECK-PURE-CODE-LONG-CALLS
+// CHECK-PURE-CODE-LONG-CALLS: error: option '-mpure-code' cannot be specified with '-mlong-calls'

cfe/trunk/test/Driver/cl-options.c

 // Note: %s must be preceded by --, otherwise it may be interpreted as a
 // command-line option, e.g. on Mac where %s is commonly under /Users.
 
 
 // Alias options:
 
 // RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck -check-prefix=c %s
 // c: -c
 
 // RUN: %clang_cl /C -### -- %s 2>&1 | FileCheck -check-prefix=C %s
-// C: error: invalid argument '-C' only allowed with '/E, /P or /EP'
+// C: error: invalid argument '/C' only allowed with '/E, /P or /EP'
 
 // RUN: %clang_cl /C /P -### -- %s 2>&1 | FileCheck -check-prefix=C_P %s
 // C_P: "-E"
 // C_P: "-C"
 
 // RUN: %clang_cl /d1reportAllClassLayout -### /c /WX -- %s 2>&1 | \
 // RUN:     FileCheck -check-prefix=d1reportAllClassLayout %s
 // d1reportAllClassLayout-NOT: warning:

 // RUN: %clang_cl /E /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
 // RUN: %clang_cl /EP /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
 // RUN: %clang_cl /E /EP /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
 // RUN: %clang_cl /EP /P /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
 // showIncludes_E-NOT: warning: argument unused during compilation: '--show-includes'
 
 // /source-charset: should warn on everything except UTF-8.
 // RUN: %clang_cl /source-charset:utf-16 -### -- %s 2>&1 | FileCheck -check-prefix=source-charset-utf-16 %s
-// source-charset-utf-16: invalid value 'utf-16'
+// source-charset-utf-16: invalid value 'utf-16' in '/source-charset:utf-16'
 
 // /execution-charset: should warn on everything except UTF-8.
 // RUN: %clang_cl /execution-charset:utf-16 -### -- %s 2>&1 | FileCheck -check-prefix=execution-charset-utf-16 %s
-// execution-charset-utf-16: invalid value 'utf-16'
+// execution-charset-utf-16: invalid value 'utf-16' in '/execution-charset:utf-16'
 //
 // RUN: %clang_cl /Umymacro -### -- %s 2>&1 | FileCheck -check-prefix=U %s
 // RUN: %clang_cl /U mymacro -### -- %s 2>&1 | FileCheck -check-prefix=U %s
 // U: "-U" "mymacro"
 
 // RUN: %clang_cl /validate-charset -### -- %s 2>&1 | FileCheck -check-prefix=validate-charset %s
 // validate-charset: -Winvalid-source-encoding
 

cfe/trunk/test/Driver/darwin-version.c

 
 // RUN: env IPHONEOS_DEPLOYMENT_TARGET=11.0 \
 // RUN:   %clang -target armv7-apple-darwin -c -### %s 2> %t.err
 // RUN:   FileCheck --input-file=%t.err --check-prefix=CHECK-VERSION-IOS4 %s
 // CHECK-VERSION-IOS4: invalid iOS deployment version 'IPHONEOS_DEPLOYMENT_TARGET=11.0'
 
 // RUN: %clang -target armv7-apple-ios11.0 -c -### %s 2> %t.err
 // RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-VERSION-IOS41 %s
-// CHECK-VERSION-IOS41: invalid iOS deployment version '--target=armv7-apple-ios11.0'
+// CHECK-VERSION-IOS41: invalid iOS deployment version '-target armv7-apple-ios11.0'
 
 // RUN: %clang -target armv7-apple-darwin -miphoneos-version-min=11.0 -c -### %s 2> %t.err
 // RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-VERSION-IOS5 %s
 // CHECK-VERSION-IOS5: invalid iOS deployment version '-miphoneos-version-min=11.0'
 
 // RUN: %clang -target i386-apple-darwin -mios-simulator-version-min=11.0 -c -### %s 2> %t.err
 // RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-VERSION-IOS6 %s
 // CHECK-VERSION-IOS6: invalid iOS deployment version '-mios-simulator-version-min=11.0'

 // RUN: %clang -target unknown-apple-darwin12  -arch armv7  -c %s -### 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-TDARWIN-FALL5 %s
 // CHECK-VERSION-TDARWIN-FALL5: "thumbv7-apple-ios5.0.0"
 
 // Warn about -m<os>-version-min when it's used with target:
 
 // RUN: %clang -target x86_64-apple-macos10.11.2 -mmacos-version-min=10.6 -c %s -### 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-TNO-OSV1 %s
-// CHECK-VERSION-TNO-OSV1: overriding '-mmacosx-version-min=10.6' option with '--target=x86_64-apple-macos10.11.2'
+// CHECK-VERSION-TNO-OSV1: overriding '-mmacos-version-min=10.6' option with '-target x86_64-apple-macos10.11.2'
 
 // RUN: %clang -target x86_64-apple-macos -miphoneos-version-min=9.1 -c %s -### 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-TNO-OSV2 %s
-// CHECK-VERSION-TNO-OSV2: overriding '-miphoneos-version-min=9.1' option with '--target=x86_64-apple-macos'
+// CHECK-VERSION-TNO-OSV2: overriding '-miphoneos-version-min=9.1' option with '-target x86_64-apple-macos'
 
 // RUN: %clang -target x86_64-apple-ios -miphonesimulator-version-min=10.0 -c %s -### 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-TNO-OSV3 %s
 // CHECK-VERSION-TNO-OSV3: "x86_64-apple-ios10.0.0-simulator"
 // CHECK-VERSION-TNO-OSV3-NOT: overriding '-mios-simulator-version-min
 // CHECK-VERSION-TNO-OSV3-NOT: argument unused during compilation
 
 // RUN: %clang -target arm64-apple-ios10.1.0 -miphoneos-version-min=10.1.0.1 -c %s -### 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-TNO-OSV4 %s
-// CHECK-VERSION-TNO-OSV4: overriding '-miphoneos-version-min=10.1.0.1' option with '--target=arm64-apple-ios10.1.0'
+// CHECK-VERSION-TNO-OSV4: overriding '-miphoneos-version-min=10.1.0.1' option with '-target arm64-apple-ios10.1.0'
 
 // RUN: %clang -target x86_64-apple-macos10.6 -mmacos-version-min=10.6 -c %s -### 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-TNO-SAME %s
 // CHECK-VERSION-TNO-SAME-NOT: overriding
 // CHECK-VERSION-TNO-SAME-NOT: argument unused during compilation
 
 // Target with OS version is not overridden by -m<os>-version-min variables:
 

lld/trunk/Common/Reproduce.cpp

 
 // Quote a given string if it contains a space character.
 std::string lld::quote(StringRef S) {
   if (S.contains(' '))
     return ("\"" + S + "\"").str();
   return S;
 }
 
+// Converts an Arg to a string representation suitable for a response file.
+// To show an Arg in a diagnostic, use Arg::getAsString() instead.
 std::string lld::toString(const opt::Arg &Arg) {
   std::string K = Arg.getSpelling();
   if (Arg.getNumValues() == 0)
     return K;
   std::string V = quote(Arg.getValue());
   if (Arg.getOption().getRenderStyle() == opt::Option::RenderJoinedStyle)
     return K + V;
   return K + " " + V;
 }

lld/trunk/ELF/Driver.cpp

static StripPolicy getStrip(opt::InputArgList &Args) {
   auto *Arg = Args.getLastArg(OPT_strip_all, OPT_strip_debug);
   if (!Arg)
     return StripPolicy::None;
   if (Arg->getOption().getID() == OPT_strip_all)
     return StripPolicy::All;
   return StripPolicy::Debug;
 }
 
-static uint64_t parseSectionAddress(StringRef S, const opt::Arg &Arg) {
+static uint64_t parseSectionAddress(StringRef S, opt::InputArgList &Args,
+                                    const opt::Arg &Arg) {
   uint64_t VA = 0;
   if (S.startswith("0x"))
     S = S.drop_front(2);
   if (!to_integer(S, VA, 16))
-    error("invalid argument: " + toString(Arg));
+    error("invalid argument: " + Arg.getAsString(Args));
   return VA;
 }
 
 static StringMap<uint64_t> getSectionStartMap(opt::InputArgList &Args) {
   StringMap<uint64_t> Ret;
   for (auto *Arg : Args.filtered(OPT_section_start)) {
     StringRef Name;
     StringRef Addr;
     std::tie(Name, Addr) = StringRef(Arg->getValue()).split('=');
-    Ret[Name] = parseSectionAddress(Addr, *Arg);
+    Ret[Name] = parseSectionAddress(Addr, Args, *Arg);
   }
 
   if (auto *Arg = Args.getLastArg(OPT_Ttext))
-    Ret[".text"] = parseSectionAddress(Arg->getValue(), *Arg);
+    Ret[".text"] = parseSectionAddress(Arg->getValue(), Args, *Arg);
   if (auto *Arg = Args.getLastArg(OPT_Tdata))
-    Ret[".data"] = parseSectionAddress(Arg->getValue(), *Arg);
+    Ret[".data"] = parseSectionAddress(Arg->getValue(), Args, *Arg);
   if (auto *Arg = Args.getLastArg(OPT_Tbss))
-    Ret[".bss"] = parseSectionAddress(Arg->getValue(), *Arg);
+    Ret[".bss"] = parseSectionAddress(Arg->getValue(), Args, *Arg);
   return Ret;
 }
 
 static SortSectionPolicy getSortSection(opt::InputArgList &Args) {
   StringRef S = Args.getLastArgValue(OPT_sort_section);
   if (S == "alignment")
     return SortSectionPolicy::Alignment;
   if (S == "name")

lld/trunk/test/ELF/sectionstart.s

 # ERR1: invalid argument: --section-start .text100000
 
 # RUN: not ld.lld %t.o --section-start .text=1Q0000 -o /dev/null 2>&1 \
 # RUN:    | FileCheck -check-prefix=ERR2 %s
 # ERR2: invalid argument: --section-start .text=1Q0000
 
 # RUN: not ld.lld %t.o -Ttext=1w0000 -o %t6 2>&1 \
 # RUN:    | FileCheck -check-prefix=ERR3 %s
-# ERR3: invalid argument: --Ttext 1w0000
+# ERR3: invalid argument: -Ttext=1w0000
 
 # RUN: not ld.lld %t.o -Tbss=1w0000 -o %t6 2>&1 \
 # RUN:    | FileCheck -check-prefix=ERR4 %s
-# ERR4: invalid argument: --Tbss 1w0000
+# ERR4: invalid argument: -Tbss=1w0000
 
 # RUN: not ld.lld %t.o -Tdata=1w0000 -o %t6 2>&1 \
 # RUN:    | FileCheck -check-prefix=ERR5 %s
-# ERR5: invalid argument: --Tdata 1w0000
+# ERR5: invalid argument: -Tdata=1w0000
 
 .text
 .globl _start
 _start:
  nop
 
 .data
 .long 0
 
 .bss
 .zero 4

llvm/trunk/include/llvm/Option/Arg.h

private:
   mutable unsigned Claimed : 1;
 
   /// Does this argument own its values?
   mutable unsigned OwnsValues : 1;
 
   /// The argument values, as C strings.
   SmallVector<const char *, 2> Values;
 
+  /// If this arg was created through an alias, this is the original alias arg.
+  /// For example, *this might be "-finput-charset=utf-8" and Alias might
+  /// point to an arg representing "/source-charset:utf-8".
+  std::unique_ptr<Arg> Alias;
+
 public:
   Arg(const Option Opt, StringRef Spelling, unsigned Index,
       const Arg *BaseArg = nullptr);
   Arg(const Option Opt, StringRef Spelling, unsigned Index,
       const char *Value0, const Arg *BaseArg = nullptr);
   Arg(const Option Opt, StringRef Spelling, unsigned Index,
       const char *Value0, const char *Value1, const Arg *BaseArg = nullptr);
   Arg(const Arg &) = delete;
public:
   ///
   /// This is either the argument itself or the argument it was
   /// derived from during tool chain specific argument translation.
   const Arg &getBaseArg() const {
     return BaseArg ? *BaseArg : *this;
   }
   void setBaseArg(const Arg *BaseArg) { this->BaseArg = BaseArg; }
 
+  /// Args are converted to their unaliased form.  For args that originally
+  /// came from an alias, this returns the alias the arg was produced from.
+  const Arg* getAlias() const { return Alias.get(); }
+  void setAlias(std::unique_ptr<Arg> Alias) { this->Alias = std::move(Alias); }
+
   bool getOwnsValues() const { return OwnsValues; }
   void setOwnsValues(bool Value) const { OwnsValues = Value; }
 
   bool isClaimed() const { return getBaseArg().Claimed; }
 
   /// Set the Arg claimed bit.
   void claim() const { getBaseArg().Claimed = true; }
 
public:
   ///
   /// The distinction is that some options only render their values
   /// when rendered as a input (e.g., Xlinker).
   void renderAsInput(const ArgList &Args, ArgStringList &Output) const;
 
   void print(raw_ostream &O) const;
   void dump() const;
 
-  /// Return a formatted version of the argument and
-  /// its values, for debugging and diagnostics.
+  /// Return a formatted version of the argument and its values, for
+  /// diagnostics. Since this is for diagnostics, if this Arg was produced
+  /// through an alias, this returns the string representation of the alias
+  /// that the user wrote.
   std::string getAsString(const ArgList &Args) const;
 };
 
 } // end namespace opt
 
 } // end namespace llvm
 
 #endif // LLVM_OPTION_ARG_H

llvm/trunk/include/llvm/Option/Option.h

public:
   /// Index to the position where argument parsing should resume
   /// (even if the argument is missing values).
   ///
   /// \param ArgSize The number of bytes taken up by the matched Option prefix
   ///                and name. This is used to determine where joined values
   ///                start.
   Arg *accept(const ArgList &Args, unsigned &Index, unsigned ArgSize) const;
 
+private:
+  Arg *acceptInternal(const ArgList &Args, unsigned &Index,
+                      unsigned ArgSize) const;
+
+public:
   void print(raw_ostream &O) const;
   void dump() const;
 };
 
 } // end namespace opt
 
 } // end namespace llvm
 
 #endif // LLVM_OPTION_OPTION_H

llvm/trunk/lib/Option/Arg.cpp

void Arg::print(raw_ostream& O) const {
   O << "]>\n";
 }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 LLVM_DUMP_METHOD void Arg::dump() const { print(dbgs()); }
 #endif
 
 std::string Arg::getAsString(const ArgList &Args) const {
+  if (Alias)
+    return Alias->getAsString(Args);
+
   SmallString<256> Res;
   raw_svector_ostream OS(Res);
 
   ArgStringList ASL;
   render(Args, ASL);
   for (ArgStringList::iterator
          it = ASL.begin(), ie = ASL.end(); it != ie; ++it) {
     if (it != ASL.begin())

llvm/trunk/lib/Option/Option.cpp

if (getID() == Opt.getID())
     return true;
 
   const Option Group = getGroup();
   if (Group.isValid())
     return Group.matches(Opt);
   return false;
 }
 
-Arg *Option::accept(const ArgList &Args,
-                    unsigned &Index,
+Arg *Option::acceptInternal(const ArgList &Args, unsigned &Index,
                             unsigned ArgSize) const {
-  const Option &UnaliasedOption = getUnaliasedOption();
-  StringRef Spelling;
-  // If the option was an alias, get the spelling from the unaliased one.
-  if (getID() == UnaliasedOption.getID()) {
-    Spelling = StringRef(Args.getArgString(Index), ArgSize);
-  } else {
-    Spelling = Args.MakeArgString(Twine(UnaliasedOption.getPrefix()) +
-                                  Twine(UnaliasedOption.getName()));
-  }
-
+  StringRef Spelling = StringRef(Args.getArgString(Index), ArgSize);
   switch (getKind()) {
   case FlagClass: {
     if (ArgSize != strlen(Args.getArgString(Index)))
       return nullptr;
-
-    Arg *A = new Arg(UnaliasedOption, Spelling, Index++);
-    if (getAliasArgs()) {
-      const char *Val = getAliasArgs();
-      while (*Val != '\0') {
-        A->getValues().push_back(Val);
-
-        // Move past the '\0' to the next argument.
-        Val += strlen(Val) + 1;
-      }
-    }
-
-    if (UnaliasedOption.getKind() == JoinedClass && !getAliasArgs())
-      // A Flag alias for a Joined option must provide an argument.
-      A->getValues().push_back("");
-
-    return A;
+    return new Arg(*this, Spelling, Index++);
   }
   case JoinedClass: {
     const char *Value = Args.getArgString(Index) + ArgSize;
-    return new Arg(UnaliasedOption, Spelling, Index++, Value);
+    return new Arg(*this, Spelling, Index++, Value);
   }
   case CommaJoinedClass: {
     // Always matches.
     const char *Str = Args.getArgString(Index) + ArgSize;
-    Arg *A = new Arg(UnaliasedOption, Spelling, Index++);
+    Arg *A = new Arg(*this, Spelling, Index++);
 
     // Parse out the comma separated values.
     const char *Prev = Str;
     for (;; ++Str) {
       char c = *Str;
 
       if (!c || c == ',') {
         if (Prev != Str) {
case SeparateClass:
     if (ArgSize != strlen(Args.getArgString(Index)))
       return nullptr;
 
     Index += 2;
     if (Index > Args.getNumInputArgStrings() ||
         Args.getArgString(Index - 1) == nullptr)
       return nullptr;
 
-    return new Arg(UnaliasedOption, Spelling,
-                   Index - 2, Args.getArgString(Index - 1));
+    return new Arg(*this, Spelling, Index - 2, Args.getArgString(Index - 1));
   case MultiArgClass: {
     // Matches iff this is an exact match.
     // FIXME: Avoid strlen.
     if (ArgSize != strlen(Args.getArgString(Index)))
       return nullptr;
 
     Index += 1 + getNumArgs();
     if (Index > Args.getNumInputArgStrings())
       return nullptr;
 
-    Arg *A = new Arg(UnaliasedOption, Spelling, Index - 1 - getNumArgs(),
-                      Args.getArgString(Index - getNumArgs()));
+    Arg *A = new Arg(*this, Spelling, Index - 1 - getNumArgs(),
     for (unsigned i = 1; i != getNumArgs(); ++i)
       A->getValues().push_back(Args.getArgString(Index - getNumArgs() + i));
     return A;
   }
   case JoinedOrSeparateClass: {
     // If this is not an exact match, it is a joined arg.
     // FIXME: Avoid strlen.
     if (ArgSize != strlen(Args.getArgString(Index))) {
       const char *Value = Args.getArgString(Index) + ArgSize;
-      return new Arg(UnaliasedOption, Spelling, Index++, Value);
+      return new Arg(*this, Spelling, Index++, Value);
     }
 
     // Otherwise it must be separate.
     Index += 2;
     if (Index > Args.getNumInputArgStrings() ||
         Args.getArgString(Index - 1) == nullptr)
       return nullptr;
 
-    return new Arg(UnaliasedOption, Spelling,
-                   Index - 2, Args.getArgString(Index - 1));
+    return new Arg(*this, Spelling, Index - 2, Args.getArgString(Index - 1));
   }
   case JoinedAndSeparateClass:
     // Always matches.
     Index += 2;
     if (Index > Args.getNumInputArgStrings() ||
         Args.getArgString(Index - 1) == nullptr)
       return nullptr;
 
-    return new Arg(UnaliasedOption, Spelling, Index - 2,
+    return new Arg(*this, Spelling, Index - 2,
                    Args.getArgString(Index - 2) + ArgSize,
                    Args.getArgString(Index - 1));
   case RemainingArgsClass: {
     // Matches iff this is an exact match.
     // FIXME: Avoid strlen.
     if (ArgSize != strlen(Args.getArgString(Index)))
       return nullptr;
-    Arg *A = new Arg(UnaliasedOption, Spelling, Index++);
+    Arg *A = new Arg(*this, Spelling, Index++);
     while (Index < Args.getNumInputArgStrings() &&
            Args.getArgString(Index) != nullptr)
       A->getValues().push_back(Args.getArgString(Index++));
     return A;
   }
   case RemainingArgsJoinedClass: {
-    Arg *A = new Arg(UnaliasedOption, Spelling, Index);
+    Arg *A = new Arg(*this, Spelling, Index);
     if (ArgSize != strlen(Args.getArgString(Index))) {
       // An inexact match means there is a joined arg.
       A->getValues().push_back(Args.getArgString(Index) + ArgSize);
     }
     Index++;
     while (Index < Args.getNumInputArgStrings() &&
            Args.getArgString(Index) != nullptr)
       A->getValues().push_back(Args.getArgString(Index++));
     return A;
   }
 
   default:
     llvm_unreachable("Invalid option kind!");
   }
 }
+
+Arg *Option::accept(const ArgList &Args,
+                    unsigned &Index,
+                    unsigned ArgSize) const {
+  std::unique_ptr<Arg> A(acceptInternal(Args, Index, ArgSize));
+  if (!A)
+    return nullptr;
+
+  const Option &UnaliasedOption = getUnaliasedOption();
+  if (getID() == UnaliasedOption.getID())
+    return A.release();
+
+  // "A" is an alias for a different flag. For most clients it's more convenient
+  // if this function returns unaliased Args, so create an unaliased arg for
+  // returning.
+
+  // This creates a completely new Arg object for the unaliased Arg because
+  // the alias and the unaliased arg can have different Kinds and different
+  // Values (due to AliasArgs<>).
+
+  // Get the spelling from the unaliased option.
+  StringRef UnaliasedSpelling = Args.MakeArgString(
+      Twine(UnaliasedOption.getPrefix()) + Twine(UnaliasedOption.getName()));
+
+  // It's a bit weird that aliased and unaliased arg share one index, but
+  // the index is mostly use as a memory optimization in render().
+  // Due to this, ArgList::getArgString(A->getIndex()) will return the spelling
+  // of the aliased arg always, while A->getSpelling() returns either the
+  // unaliased or the aliased arg, depending on which Arg object it's called on.
+  Arg *UnaliasedA = new Arg(UnaliasedOption, UnaliasedSpelling, A->getIndex());
+  Arg *RawA = A.get();
+  UnaliasedA->setAlias(std::move(A));
+
+  if (getKind() != FlagClass) {
+    // Values are usually owned by the ArgList. The exception are
+    // CommaJoined flags, where the Arg owns the values. For aliased flags,
+    // make the unaliased Arg the owner of the values.
+    // FIXME: There aren't many uses of CommaJoined -- try removing
+    // CommaJoined in favor of just calling StringRef::split(',') instead.
+    UnaliasedA->getValues() = RawA->getValues();
+    UnaliasedA->setOwnsValues(RawA->getOwnsValues());
+    RawA->setOwnsValues(false);
+    return UnaliasedA;
+  }
+
+  // FlagClass aliases can have AliasArgs<>; add those to the unaliased arg.
+  if (const char *Val = getAliasArgs()) {
+    while (*Val != '\0') {
+      UnaliasedA->getValues().push_back(Val);
+
+      // Move past the '\0' to the next argument.
+      Val += strlen(Val) + 1;
+    }
+  }
+  if (UnaliasedOption.getKind() == JoinedClass && !getAliasArgs())
+    // A Flag alias for a Joined option must provide an argument.
+    UnaliasedA->getValues().push_back("");
+  return UnaliasedA;
+}