thakis created this revision.
thakis added a reviewer: ruiu.
Herald added subscribers: MaskRay, aheejin, hiraditya, arichardson, sbc100, 
emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.

This fixes an 8-year-old regression. r105763 made it so that aliases always 
refer to the unaliased option – but it missed the "joined" branch of 
JoinedOrSeparate flags. (r162231 then made the Args classes non-virtual, and 
r169344 moved them from clang to llvm.)

Back then, there was no JoinedOrSeparate flag that was an alias, so it wasn't 
observable. Now /U in CLCompatOptions is a JoinedOrSeparate alias in clang, and 
warn_slash_u_filename incorrectly used the aliased arg id (using the unaliased 
one isn't really a regression since that warning checks if the undefined macro 
contains slash or backslash and only then emits the warning – and no valid use 
will pass "-Ufoo/bar" or similar).

Also, lld has many JoinedOrSeparate aliases, and due to this bug it had to 
explicitly call `getUnaliasedOption()` in a bunch of places, even though that 
shouldn't be necessary by design. After this fix in Option, these calls really 
don't have an effect any more, so remove them.

No intended behavior change.

(I accidentally fixed this bug while working on PR29106 but then wondered why 
the warn_slash_u_filename broke. When I figured it out, I thought it would make 
sense to land this in a separate commit.)


https://reviews.llvm.org/D64156

Files:
  clang/lib/Driver/Driver.cpp
  lld/COFF/Driver.cpp
  lld/ELF/Driver.cpp
  lld/ELF/DriverUtils.cpp
  lld/MinGW/Driver.cpp
  lld/wasm/Driver.cpp
  llvm/lib/Option/Option.cpp


Index: llvm/lib/Option/Option.cpp
===================================================================
--- llvm/lib/Option/Option.cpp
+++ llvm/lib/Option/Option.cpp
@@ -207,7 +207,7 @@
     // FIXME: Avoid strlen.
     if (ArgSize != strlen(Args.getArgString(Index))) {
       const char *Value = Args.getArgString(Index) + ArgSize;
-      return new Arg(*this, Spelling, Index++, Value);
+      return new Arg(UnaliasedOption, Spelling, Index++, Value);
     }
 
     // Otherwise it must be separate.
Index: lld/wasm/Driver.cpp
===================================================================
--- lld/wasm/Driver.cpp
+++ lld/wasm/Driver.cpp
@@ -271,7 +271,7 @@
 
 void LinkerDriver::createFiles(opt::InputArgList &Args) {
   for (auto *Arg : Args) {
-    switch (Arg->getOption().getUnaliasedOption().getID()) {
+    switch (Arg->getOption().getID()) {
     case OPT_l:
       addLibrary(Arg->getValue());
       break;
@@ -519,7 +519,7 @@
 
   // Copy the command line to the output while rewriting paths.
   for (auto *Arg : Args) {
-    switch (Arg->getOption().getUnaliasedOption().getID()) {
+    switch (Arg->getOption().getID()) {
     case OPT_reproduce:
       break;
     case OPT_INPUT:
Index: lld/MinGW/Driver.cpp
===================================================================
--- lld/MinGW/Driver.cpp
+++ lld/MinGW/Driver.cpp
@@ -313,7 +313,7 @@
   StringRef Prefix = "";
   bool Static = false;
   for (auto *A : Args) {
-    switch (A->getOption().getUnaliasedOption().getID()) {
+    switch (A->getOption().getID()) {
     case OPT_INPUT:
       if (StringRef(A->getValue()).endswith_lower(".def"))
         Add("-def:" + StringRef(A->getValue()));
Index: lld/ELF/DriverUtils.cpp
===================================================================
--- lld/ELF/DriverUtils.cpp
+++ lld/ELF/DriverUtils.cpp
@@ -172,7 +172,7 @@
 
   // Copy the command line to the output while rewriting paths.
   for (auto *Arg : Args) {
-    switch (Arg->getOption().getUnaliasedOption().getID()) {
+    switch (Arg->getOption().getID()) {
     case OPT_reproduce:
       break;
     case OPT_INPUT:
Index: lld/ELF/Driver.cpp
===================================================================
--- lld/ELF/Driver.cpp
+++ lld/ELF/Driver.cpp
@@ -1110,7 +1110,7 @@
 
   // Iterate over argv to process input files and positional arguments.
   for (auto *Arg : Args) {
-    switch (Arg->getOption().getUnaliasedOption().getID()) {
+    switch (Arg->getOption().getID()) {
     case OPT_library:
       addLibrary(Arg->getValue());
       break;
Index: lld/COFF/Driver.cpp
===================================================================
--- lld/COFF/Driver.cpp
+++ lld/COFF/Driver.cpp
@@ -332,7 +332,7 @@
   }
 
   for (auto *Arg : Args) {
-    switch (Arg->getOption().getUnaliasedOption().getID()) {
+    switch (Arg->getOption().getID()) {
     case OPT_aligncomm:
       parseAligncomm(Arg->getValue());
       break;
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2180,7 +2180,7 @@
         Diag(clang::diag::err_drv_unknown_language) << A->getValue();
         InputType = types::TY_Object;
       }
-    } else if (A->getOption().getID() == options::OPT__SLASH_U) {
+    } else if (A->getOption().getID() == options::OPT_U) {
       assert(A->getNumValues() == 1 && "The /U option has one value.");
       StringRef Val = A->getValue(0);
       if (Val.find_first_of("/\\") != StringRef::npos) {


Index: llvm/lib/Option/Option.cpp
===================================================================
--- llvm/lib/Option/Option.cpp
+++ llvm/lib/Option/Option.cpp
@@ -207,7 +207,7 @@
     // FIXME: Avoid strlen.
     if (ArgSize != strlen(Args.getArgString(Index))) {
       const char *Value = Args.getArgString(Index) + ArgSize;
-      return new Arg(*this, Spelling, Index++, Value);
+      return new Arg(UnaliasedOption, Spelling, Index++, Value);
     }
 
     // Otherwise it must be separate.
Index: lld/wasm/Driver.cpp
===================================================================
--- lld/wasm/Driver.cpp
+++ lld/wasm/Driver.cpp
@@ -271,7 +271,7 @@
 
 void LinkerDriver::createFiles(opt::InputArgList &Args) {
   for (auto *Arg : Args) {
-    switch (Arg->getOption().getUnaliasedOption().getID()) {
+    switch (Arg->getOption().getID()) {
     case OPT_l:
       addLibrary(Arg->getValue());
       break;
@@ -519,7 +519,7 @@
 
   // Copy the command line to the output while rewriting paths.
   for (auto *Arg : Args) {
-    switch (Arg->getOption().getUnaliasedOption().getID()) {
+    switch (Arg->getOption().getID()) {
     case OPT_reproduce:
       break;
     case OPT_INPUT:
Index: lld/MinGW/Driver.cpp
===================================================================
--- lld/MinGW/Driver.cpp
+++ lld/MinGW/Driver.cpp
@@ -313,7 +313,7 @@
   StringRef Prefix = "";
   bool Static = false;
   for (auto *A : Args) {
-    switch (A->getOption().getUnaliasedOption().getID()) {
+    switch (A->getOption().getID()) {
     case OPT_INPUT:
       if (StringRef(A->getValue()).endswith_lower(".def"))
         Add("-def:" + StringRef(A->getValue()));
Index: lld/ELF/DriverUtils.cpp
===================================================================
--- lld/ELF/DriverUtils.cpp
+++ lld/ELF/DriverUtils.cpp
@@ -172,7 +172,7 @@
 
   // Copy the command line to the output while rewriting paths.
   for (auto *Arg : Args) {
-    switch (Arg->getOption().getUnaliasedOption().getID()) {
+    switch (Arg->getOption().getID()) {
     case OPT_reproduce:
       break;
     case OPT_INPUT:
Index: lld/ELF/Driver.cpp
===================================================================
--- lld/ELF/Driver.cpp
+++ lld/ELF/Driver.cpp
@@ -1110,7 +1110,7 @@
 
   // Iterate over argv to process input files and positional arguments.
   for (auto *Arg : Args) {
-    switch (Arg->getOption().getUnaliasedOption().getID()) {
+    switch (Arg->getOption().getID()) {
     case OPT_library:
       addLibrary(Arg->getValue());
       break;
Index: lld/COFF/Driver.cpp
===================================================================
--- lld/COFF/Driver.cpp
+++ lld/COFF/Driver.cpp
@@ -332,7 +332,7 @@
   }
 
   for (auto *Arg : Args) {
-    switch (Arg->getOption().getUnaliasedOption().getID()) {
+    switch (Arg->getOption().getID()) {
     case OPT_aligncomm:
       parseAligncomm(Arg->getValue());
       break;
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2180,7 +2180,7 @@
         Diag(clang::diag::err_drv_unknown_language) << A->getValue();
         InputType = types::TY_Object;
       }
-    } else if (A->getOption().getID() == options::OPT__SLASH_U) {
+    } else if (A->getOption().getID() == options::OPT_U) {
       assert(A->getNumValues() == 1 && "The /U option has one value.");
       StringRef Val = A->getValue(0);
       if (Val.find_first_of("/\\") != StringRef::npos) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to