[PATCH] D145567: [Driver] Rename multilib flags to tags

[Michael Platings via Phabricator via cfe-commits]

michaelplatings abandoned this revision.
michaelplatings marked an inline comment as done.
michaelplatings added a comment.

We're going back to "flags": https://discourse.llvm.org/t/rfc-multilib/67494/14


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145567/new/

https://reviews.llvm.org/D145567

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145567: [Driver] Rename multilib flags to tags

[Michael Platings via Phabricator via cfe-commits]

michaelplatings marked an inline comment as done.
michaelplatings added a comment.

Thanks @peter.smith. I've opted to leave the comment as-is. If we can expect a 
tag_set to actually contain flags then I've continued to use that terminology.




Comment at: clang/lib/Driver/ToolChains/CommonArgs.h:202
+ Multilib::tag_set );
 
 void addX86AlignBranchArgs(const Driver , const llvm::opt::ArgList ,

peter.smith wrote:
> I can see the reason to keep the name `addMultilibFlag`. At this point is the 
> tag_set expected to be simplified tags or full command line flags. If it is 
> the former I think it would be good to change Flags to Tags here.
> 
> May also be useul to add a \p for Flags (or Tags) if there are any 
> requirements, or just useful information on what it is expected to be.
> 
> Parameter name also applies to CommonArgs.cpp below.
For the way this function is used I would expect `Flags` to be command line 
flags prefixed with `+` or `-`, the same as for the `Flag` parameter. Therefore 
I think it best to leave it as-is.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145567/new/

https://reviews.llvm.org/D145567

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145567: [Driver] Rename multilib flags to tags

[Peter Smith via Phabricator via cfe-commits]

peter.smith accepted this revision.
peter.smith added a comment.

Actually click the button this time to set approval, see previous comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145567/new/

https://reviews.llvm.org/D145567

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145567: [Driver] Rename multilib flags to tags

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

I've set approved from the Arm side. Please leave some time for people in the 
US time zone to leave any final comments or ask for extensions. 
From looking at the source code alone - assuming that most people would not 
track down the commit message/review for extra context - I found it difficult 
to work out the convention for when Flags is used and when Tags is used. I've 
made a suggestion for some comments. This can be applied prior to commit if you 
want to take it up.




Comment at: clang/lib/Driver/ToolChains/CommonArgs.h:202
+ Multilib::tag_set );
 
 void addX86AlignBranchArgs(const Driver , const llvm::opt::ArgList ,

I can see the reason to keep the name `addMultilibFlag`. At this point is the 
tag_set expected to be simplified tags or full command line flags. If it is the 
former I think it would be good to change Flags to Tags here.

May also be useul to add a \p for Flags (or Tags) if there are any 
requirements, or just useful information on what it is expected to be.

Parameter name also applies to CommonArgs.cpp below.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145567/new/

https://reviews.llvm.org/D145567

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145567: [Driver] Rename multilib flags to tags

[Michael Platings via Phabricator via cfe-commits]

michaelplatings updated this revision to Diff 503913.
michaelplatings added a comment.

Tiny tweak: undo an unnecessary change to a test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145567/new/

https://reviews.llvm.org/D145567

Files:
  clang/include/clang/Driver/Multilib.h
  clang/lib/Driver/Multilib.cpp
  clang/lib/Driver/MultilibBuilder.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/unittests/Driver/MultilibBuilderTest.cpp
  clang/unittests/Driver/MultilibTest.cpp

Index: clang/unittests/Driver/MultilibTest.cpp
===
--- clang/unittests/Driver/MultilibTest.cpp
+++ clang/unittests/Driver/MultilibTest.cpp
@@ -35,21 +35,21 @@
 TEST(MultilibTest, OpEqReflexivity3) {
   Multilib M1({}, {}, {}, {"+foo"});
   Multilib M2({}, {}, {}, {"+foo"});
-  ASSERT_TRUE(M1 == M2) << "Multilibs with the same flag should be the same";
+  ASSERT_TRUE(M1 == M2) << "Multilibs with the same tag should be the same";
 }
 
 TEST(MultilibTest, OpEqInequivalence1) {
   Multilib M1({}, {}, {}, {"+foo"});
   Multilib M2({}, {}, {}, {"-foo"});
-  ASSERT_FALSE(M1 == M2) << "Multilibs with conflicting flags are not the same";
+  ASSERT_FALSE(M1 == M2) << "Multilibs with conflicting tags are not the same";
   ASSERT_FALSE(M2 == M1)
-  << "Multilibs with conflicting flags are not the same (commuted)";
+  << "Multilibs with conflicting tags are not the same (commuted)";
 }
 
 TEST(MultilibTest, OpEqInequivalence2) {
   Multilib M1;
   Multilib M2({}, {}, {}, {"+foo"});
-  ASSERT_FALSE(M1 == M2) << "Flags make Multilibs different";
+  ASSERT_FALSE(M1 == M2) << "Tags make Multilibs different";
 }
 
 TEST(MultilibTest, OpEqEquivalence2) {
@@ -125,8 +125,8 @@
 
 TEST(MultilibTest, Construction3) {
   Multilib M({}, {}, {}, {"+f1", "+f2", "-f3"});
-  for (Multilib::flag_set::const_iterator I = M.flags().begin(),
-  E = M.flags().end();
+  for (Multilib::tag_set::const_iterator I = M.tags().begin(),
+ E = M.tags().end();
I != E; ++I) {
 ASSERT_TRUE(llvm::StringSwitch(*I)
 .Cases("+f1", "+f2", "-f3", true)
@@ -152,17 +152,17 @@
   Multilib("/foo", {}, {}, {"+foo"}),
   Multilib("/bar", {}, {}, {"+bar"}),
   });
-  Multilib::flag_set Flags1 = {"+foo", "-bar"};
+  Multilib::tag_set Tags1 = {"+foo", "-bar"};
   Multilib Selection1;
-  ASSERT_TRUE(MS.select(Flags1, Selection1))
-  << "Flag set was {\"+foo\"}, but selection not found";
+  ASSERT_TRUE(MS.select(Tags1, Selection1))
+  << "Tag set was {\"+foo\"}, but selection not found";
   ASSERT_TRUE(Selection1.gccSuffix() == "/foo")
   << "Selection picked " << Selection1 << " which was not expected";
 
-  Multilib::flag_set Flags2 = {"+foo", "+bar"};
+  Multilib::tag_set Tags2 = {"+foo", "+bar"};
   Multilib Selection2;
-  ASSERT_TRUE(MS.select(Flags2, Selection2))
-  << "Flag set was {\"+bar\"}, but selection not found";
+  ASSERT_TRUE(MS.select(Tags2, Selection2))
+  << "Tag set was {\"+bar\"}, but selection not found";
   ASSERT_TRUE(Selection2.gccSuffix() == "/bar")
   << "Selection picked " << Selection2 << " which was not expected";
 }
Index: clang/unittests/Driver/MultilibBuilderTest.cpp
===
--- clang/unittests/Driver/MultilibBuilderTest.cpp
+++ clang/unittests/Driver/MultilibBuilderTest.cpp
@@ -68,9 +68,9 @@
   ASSERT_TRUE(MS.size() == 2);
   for (MultilibSet::const_iterator I = MS.begin(), E = MS.end(); I != E; ++I) {
 if (I->gccSuffix() == "/64")
-  ASSERT_TRUE(*I->flags().begin() == "+m64");
+  ASSERT_TRUE(*I->tags().begin() == "+m64");
 else if (I->gccSuffix() == "")
-  ASSERT_TRUE(*I->flags().begin() == "-m64");
+  ASSERT_TRUE(*I->tags().begin() == "-m64");
 else
   FAIL() << "Unrecognized gccSufix: " << I->gccSuffix();
   }
@@ -89,17 +89,17 @@
 .Default(false))
 << "Multilib " << *I << " wasn't expected";
 ASSERT_TRUE(llvm::StringSwitch(I->gccSuffix())
-.Case("", is_contained(I->flags(), "-sof"))
-.Case("/sof", is_contained(I->flags(), "+sof"))
-.Case("/el", is_contained(I->flags(), "-sof"))
-.Case("/sof/el", is_contained(I->flags(), "+sof"))
+.Case("", is_contained(I->tags(), "-sof"))
+.Case("/sof", is_contained(I->tags(), "+sof"))
+.Case("/el", is_contained(I->tags(), "-sof"))
+.Case("/sof/el", is_contained(I->tags(), "+sof"))
 .Default(false))
 << "Multilib " << *I << " didn't have the appropriate {+,-}sof flag";
 

[PATCH] D145567: [Driver] Rename multilib flags to tags

[Michael Platings via Phabricator via cfe-commits]

michaelplatings updated this revision to Diff 503899.
michaelplatings added a comment.

Rebase on top of D142905 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145567/new/

https://reviews.llvm.org/D145567

Files:
  clang/include/clang/Driver/Multilib.h
  clang/lib/Driver/Multilib.cpp
  clang/lib/Driver/MultilibBuilder.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/unittests/Driver/MultilibBuilderTest.cpp
  clang/unittests/Driver/MultilibTest.cpp

Index: clang/unittests/Driver/MultilibTest.cpp
===
--- clang/unittests/Driver/MultilibTest.cpp
+++ clang/unittests/Driver/MultilibTest.cpp
@@ -35,21 +35,21 @@
 TEST(MultilibTest, OpEqReflexivity3) {
   Multilib M1({}, {}, {}, {"+foo"});
   Multilib M2({}, {}, {}, {"+foo"});
-  ASSERT_TRUE(M1 == M2) << "Multilibs with the same flag should be the same";
+  ASSERT_TRUE(M1 == M2) << "Multilibs with the same tag should be the same";
 }
 
 TEST(MultilibTest, OpEqInequivalence1) {
   Multilib M1({}, {}, {}, {"+foo"});
   Multilib M2({}, {}, {}, {"-foo"});
-  ASSERT_FALSE(M1 == M2) << "Multilibs with conflicting flags are not the same";
+  ASSERT_FALSE(M1 == M2) << "Multilibs with conflicting tags are not the same";
   ASSERT_FALSE(M2 == M1)
-  << "Multilibs with conflicting flags are not the same (commuted)";
+  << "Multilibs with conflicting tags are not the same (commuted)";
 }
 
 TEST(MultilibTest, OpEqInequivalence2) {
   Multilib M1;
   Multilib M2({}, {}, {}, {"+foo"});
-  ASSERT_FALSE(M1 == M2) << "Flags make Multilibs different";
+  ASSERT_FALSE(M1 == M2) << "Tags make Multilibs different";
 }
 
 TEST(MultilibTest, OpEqEquivalence2) {
@@ -125,8 +125,8 @@
 
 TEST(MultilibTest, Construction3) {
   Multilib M({}, {}, {}, {"+f1", "+f2", "-f3"});
-  for (Multilib::flag_set::const_iterator I = M.flags().begin(),
-  E = M.flags().end();
+  for (Multilib::tag_set::const_iterator I = M.tags().begin(),
+ E = M.tags().end();
I != E; ++I) {
 ASSERT_TRUE(llvm::StringSwitch(*I)
 .Cases("+f1", "+f2", "-f3", true)
@@ -152,17 +152,17 @@
   Multilib("/foo", {}, {}, {"+foo"}),
   Multilib("/bar", {}, {}, {"+bar"}),
   });
-  Multilib::flag_set Flags1 = {"+foo", "-bar"};
+  Multilib::tag_set Tags1 = {"+foo", "-bar"};
   Multilib Selection1;
-  ASSERT_TRUE(MS.select(Flags1, Selection1))
-  << "Flag set was {\"+foo\"}, but selection not found";
+  ASSERT_TRUE(MS.select(Tags1, Selection1))
+  << "Tag set was {\"+foo\"}, but selection not found";
   ASSERT_TRUE(Selection1.gccSuffix() == "/foo")
   << "Selection picked " << Selection1 << " which was not expected";
 
-  Multilib::flag_set Flags2 = {"+foo", "+bar"};
+  Multilib::tag_set Tags2 = {"+foo", "+bar"};
   Multilib Selection2;
-  ASSERT_TRUE(MS.select(Flags2, Selection2))
-  << "Flag set was {\"+bar\"}, but selection not found";
+  ASSERT_TRUE(MS.select(Tags2, Selection2))
+  << "Tag set was {\"+bar\"}, but selection not found";
   ASSERT_TRUE(Selection2.gccSuffix() == "/bar")
   << "Selection picked " << Selection2 << " which was not expected";
 }
Index: clang/unittests/Driver/MultilibBuilderTest.cpp
===
--- clang/unittests/Driver/MultilibBuilderTest.cpp
+++ clang/unittests/Driver/MultilibBuilderTest.cpp
@@ -34,7 +34,7 @@
   << "Single contraindicative flag is not valid";
 
   ASSERT_FALSE(MultilibBuilder().flag("+foo").flag("-foo").isValid())
-  << "Conflicting flags should invalidate the Multilib";
+  << "Conflicting tags should invalidate the Multilib";
 
   ASSERT_TRUE(MultilibBuilder().flag("+foo").flag("+foo").isValid())
   << "Multilib should be valid even if it has the same flag "
@@ -68,9 +68,9 @@
   ASSERT_TRUE(MS.size() == 2);
   for (MultilibSet::const_iterator I = MS.begin(), E = MS.end(); I != E; ++I) {
 if (I->gccSuffix() == "/64")
-  ASSERT_TRUE(*I->flags().begin() == "+m64");
+  ASSERT_TRUE(*I->tags().begin() == "+m64");
 else if (I->gccSuffix() == "")
-  ASSERT_TRUE(*I->flags().begin() == "-m64");
+  ASSERT_TRUE(*I->tags().begin() == "-m64");
 else
   FAIL() << "Unrecognized gccSufix: " << I->gccSuffix();
   }
@@ -89,17 +89,17 @@
 .Default(false))
 << "Multilib " << *I << " wasn't expected";
 ASSERT_TRUE(llvm::StringSwitch(I->gccSuffix())
-.Case("", is_contained(I->flags(), "-sof"))
-.Case("/sof", is_contained(I->flags(), "+sof"))
-.Case("/el", is_contained(I->flags(), "-sof"))
-.Case("/sof/el", is_contained(I->flags(), 

[PATCH] D145567: [Driver] Rename multilib flags to tags

[Michael Platings via Phabricator via cfe-commits]

michaelplatings added a comment.

Thanks @MaskRay for taking a look and thanks @simon_tatham for the review of 
the change. This change affects existing code so I think it deserves its own 
commit, but I'll move it down the stack to before D142932 
 so that later changes use the new names 
immediately, and I'll incorporate Simon's suggestions into those.




Comment at: clang/docs/Multilib.rst:66
``--target=armv7m-none-eabi`` are equivalent. Clang can also accept many
-   independent pieces of information within a single flag - for example
+   independent pieces of information within a single option - for example
``-march=armv8.1m.main+fp+mve`` specifies the architecture and two

simon_tatham wrote:
> An "option" here seems to be the same thing as an "argument" elsewhere in 
> this paragraph. Since the terminology is already confusing, perhaps simplify 
> by using the same word consistently throughout? I think "option" is more 
> precise, because //positional// clang arguments like input files definitely 
> //don't// play a part in this mechanism.
I'll incorporate this into D143587.



Comment at: clang/docs/Multilib.rst:71
+   arguments into a standard set of simpler "tags". In many cases these tags
will look like a command line argument with the leading ``-`` stripped off,
+   but where a suitable form for the tag doesn't exist in command line

simon_tatham wrote:
> This is a particular case where "option" seems like a better word. Not every 
> //argument// has a leading `-` in the first place. But every //option// does.
> 
> (Or, at least, in the default Unix / gcc style of clang options. I suppose in 
> `clang-cl` even that is not true, because options can have a leading `/` in 
> Windows style. I assume that in that situation the options are normalised to 
> their GNU representation before converting into multilib selection tags?)
I'll incorporate this into D143587.



Comment at: clang/docs/Multilib.rst:182
   # List of multilib variants. Required.
   # The ordering of Variants is important if more than one variant can match
+  # the same set of tags. See the docs on multilib layering for more info.

simon_tatham wrote:
> That capital V looks unintentional to me, and is potentially confusing – 
> someone might go looking for a formal definition of it somewhere.
I'll incorporate this into D143587.



Comment at: clang/include/clang/Driver/Multilib.h:64-65
+  /// Get the set of tags that indicate this multilib's use.
+  /// Tags are arbitrary strings although typically they will look similar to
+  /// command line options. A multilib is considered compatible if its tags are
+  /// a subset of the tags derived from the Clang command line options.

simon_tatham wrote:
> Tags are arbitrary strings, some of which are derived from command-line 
> options and look similar to them, and others can be defined by a particular 
> multilib.yaml
I'll move this change to earlier in the stack before multilib.yaml is a thing, 
but then I'll update the comment with your suggestion in D142932.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145567/new/

https://reviews.llvm.org/D145567

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145567: [Driver] Rename multilib flags to tags

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added inline comments.



Comment at: clang/docs/Multilib.rst:71
+   arguments into a standard set of simpler "tags". In many cases these tags
will look like a command line argument with the leading ``-`` stripped off,
+   but where a suitable form for the tag doesn't exist in command line

This is a particular case where "option" seems like a better word. Not every 
//argument// has a leading `-` in the first place. But every //option// does.

(Or, at least, in the default Unix / gcc style of clang options. I suppose in 
`clang-cl` even that is not true, because options can have a leading `/` in 
Windows style. I assume that in that situation the options are normalised to 
their GNU representation before converting into multilib selection tags?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145567/new/

https://reviews.llvm.org/D145567

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145567: [Driver] Rename multilib flags to tags

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added a comment.

But it was useful to have it separate at least for review purposes, since it 
made it much easier when Michael asked me to proofread the change from 'flags' 
to 'tags'!




Comment at: clang/docs/Multilib.rst:66
``--target=armv7m-none-eabi`` are equivalent. Clang can also accept many
-   independent pieces of information within a single flag - for example
+   independent pieces of information within a single option - for example
``-march=armv8.1m.main+fp+mve`` specifies the architecture and two

An "option" here seems to be the same thing as an "argument" elsewhere in this 
paragraph. Since the terminology is already confusing, perhaps simplify by 
using the same word consistently throughout? I think "option" is more precise, 
because //positional// clang arguments like input files definitely //don't// 
play a part in this mechanism.



Comment at: clang/docs/Multilib.rst:182
   # List of multilib variants. Required.
   # The ordering of Variants is important if more than one variant can match
+  # the same set of tags. See the docs on multilib layering for more info.

That capital V looks unintentional to me, and is potentially confusing – 
someone might go looking for a formal definition of it somewhere.



Comment at: clang/include/clang/Driver/Multilib.h:64-65
+  /// Get the set of tags that indicate this multilib's use.
+  /// Tags are arbitrary strings although typically they will look similar to
+  /// command line options. A multilib is considered compatible if its tags are
+  /// a subset of the tags derived from the Clang command line options.

Tags are arbitrary strings, some of which are derived from command-line options 
and look similar to them, and others can be defined by a particular 
multilib.yaml


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145567/new/

https://reviews.llvm.org/D145567

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145567: [Driver] Rename multilib flags to tags

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Since the touched code is still in review, you need to merge the changes into 
the prior patches.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145567/new/

https://reviews.llvm.org/D145567

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145567: [Driver] Rename multilib flags to tags

[Michael Platings via Phabricator via cfe-commits]

michaelplatings created this revision.
michaelplatings added a reviewer: phosek.
Herald added a subscriber: abrachet.
Herald added a project: All.
michaelplatings requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

The name "flag" implies a type of command line option which is no longer
how these variables are used in multilib. Although tags often look
similar to command line options this is not a requirement - tags may be
arbitrary strings.

MultilibBuilder still uses the name "flag" since it is designed to be
used only with valid command line options, even though these are
subsequently used as multilib tags. Similarly addMultilibFlag() keeps
its existing name because of how it is used, even though it now takes a
tag_set argument.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145567

Files:
  clang/docs/Multilib.rst
  clang/include/clang/Driver/Multilib.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Multilib.cpp
  clang/lib/Driver/MultilibBuilder.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/baremetal-multilib.yaml
  clang/test/Driver/print-multi-selection-flags.c
  clang/test/Driver/print-multi-selection-tags.c
  clang/unittests/Driver/MultilibBuilderTest.cpp
  clang/unittests/Driver/MultilibTest.cpp

Index: clang/unittests/Driver/MultilibTest.cpp
===
--- clang/unittests/Driver/MultilibTest.cpp
+++ clang/unittests/Driver/MultilibTest.cpp
@@ -36,21 +36,21 @@
 TEST(MultilibTest, OpEqReflexivity3) {
   Multilib M1({}, {}, {}, {"+foo"});
   Multilib M2({}, {}, {}, {"+foo"});
-  ASSERT_TRUE(M1 == M2) << "Multilibs with the same flag should be the same";
+  ASSERT_TRUE(M1 == M2) << "Multilibs with the same tag should be the same";
 }
 
 TEST(MultilibTest, OpEqInequivalence1) {
   Multilib M1({}, {}, {}, {"+foo"});
   Multilib M2({}, {}, {}, {"-foo"});
-  ASSERT_FALSE(M1 == M2) << "Multilibs with conflicting flags are not the same";
+  ASSERT_FALSE(M1 == M2) << "Multilibs with conflicting tags are not the same";
   ASSERT_FALSE(M2 == M1)
-  << "Multilibs with conflicting flags are not the same (commuted)";
+  << "Multilibs with conflicting tags are not the same (commuted)";
 }
 
 TEST(MultilibTest, OpEqInequivalence2) {
   Multilib M1;
   Multilib M2({}, {}, {}, {"+foo"});
-  ASSERT_FALSE(M1 == M2) << "Flags make Multilibs different";
+  ASSERT_FALSE(M1 == M2) << "Tags make Multilibs different";
 }
 
 TEST(MultilibTest, OpEqEquivalence2) {
@@ -126,8 +126,8 @@
 
 TEST(MultilibTest, Construction3) {
   Multilib M({}, {}, {}, {"+f1", "+f2", "-f3"});
-  for (Multilib::flag_set::const_iterator I = M.flags().begin(),
-  E = M.flags().end();
+  for (Multilib::tag_set::const_iterator I = M.tags().begin(),
+  E = M.tags().end();
I != E; ++I) {
 ASSERT_TRUE(llvm::StringSwitch(*I)
 .Cases("+f1", "+f2", "-f3", true)
@@ -153,17 +153,17 @@
   Multilib("/foo", {}, {}, {"+foo"}),
   Multilib("/bar", {}, {}, {"+bar"}),
   });
-  Multilib::flag_set Flags1 = {"+foo", "-bar"};
+  Multilib::tag_set Tags1 = {"+foo", "-bar"};
   llvm::SmallVector Selection1;
-  ASSERT_TRUE(MS.select(Flags1, Selection1))
-  << "Flag set was {\"+foo\"}, but selection not found";
+  ASSERT_TRUE(MS.select(Tags1, Selection1))
+  << "Tag set was {\"+foo\"}, but selection not found";
   ASSERT_TRUE(Selection1.back().gccSuffix() == "/foo")
   << "Selection picked " << Selection1.back() << " which was not expected";
 
-  Multilib::flag_set Flags2 = {"+foo", "+bar"};
+  Multilib::tag_set Tags2 = {"+foo", "+bar"};
   llvm::SmallVector Selection2;
-  ASSERT_TRUE(MS.select(Flags2, Selection2))
-  << "Flag set was {\"+bar\"}, but selection not found";
+  ASSERT_TRUE(MS.select(Tags2, Selection2))
+  << "Tag set was {\"+bar\"}, but selection not found";
   ASSERT_TRUE(Selection2.back().gccSuffix() == "/bar")
   << "Selection picked " << Selection2.back() << " which was not expected";
 }
@@ -276,7 +276,7 @@
   EXPECT_FALSE(parse(MS, Diagnostic, YAML_PREAMBLE R"(
 Variants:
 - Dir: /abc
-  Flags: []
+  Tags: []
   PrintOptions: []
 )"));
   EXPECT_TRUE(StringRef(Diagnostic).contains("paths must be relative"))
@@ -284,7 +284,7 @@
 
   EXPECT_FALSE(parse(MS, Diagnostic, YAML_PREAMBLE R"(
 Variants:
-- Flags: []
+- Tags: []
   PrintOptions: []
 )"));
   EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'Dir'"))
@@ -295,13 +295,13 @@
 - Dir: .
   PrintOptions: []
 )"));
-  EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'Flags'"))
+