[NFC] [FileCheck] Fix init of objects in unit tests

[thopre]

Summary

Fix initialization style of objects allocated on the stack and member
objects in unit test to use the "Type Var(init list)" convention.

[jhenderson]

LGTM, since it makes things more readable. Beyond that, is there any particular motivation for this?

[grimar]

LGTM, this makes the code cleaner.
(btw, you can add "NFC" to the title it seems).

[thopre]

In D68425#1694261, @jhenderson wrote:

LGTM, since it makes things more readable. Beyond that, is there any particular motivation for this?

Several comments in https://reviews.llvm.org/D60389 asking to fix the same issue. Since there was already several instances of the issue in existing code I thought I'd fix them.

[gribozavr]

Sorry, but this change broke the build: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/18721

Please always run ninja check-all before committing.

I reverted this change in r373722.

[thopre]

In D68425#1694398, @gribozavr wrote:

Sorry, but this change broke the build: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/18721

Please always run ninja check-all before committing.

I reverted this change in r373722.

I did ran check check-llvm-unit which runs this test. Is it possible that the unit test is not run using the compiler just built?

[thopre]

In D68425#1694571, @thopre wrote:

In D68425#1694398, @gribozavr wrote:

Sorry, but this change broke the build: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/18721

Please always run ninja check-all before committing.

I reverted this change in r373722.

I did ran check check-llvm-unit which runs this test. Is it possible that the unit test is not run using the compiler just built?

My bad, I can reproduce this, I might have just run ninja which of course does not do anything here.

[thopre]

Fix syntax for member initialization

[thopre]

Ok to commit this revised change? I double checked that I ran the unit tests this time and all tests pass.

[probinson]

In D68425#1694675, @thopre wrote:

Ok to commit this revised change? I double checked that I ran the unit tests this time and all tests pass.

For simple changes to fix a bot breakage, it's generally fine to just recommit the updated fix; people also like to see a comment in the commit log for the recommit that says something about what's different this time or why it's okay to recommit.

[thopre]

In D68425#1694745, @probinson wrote:

In D68425#1694675, @thopre wrote:

Ok to commit this revised change? I double checked that I ran the unit tests this time and all tests pass.

For simple changes to fix a bot breakage, it's generally fine to just recommit the updated fix; people also like to see a comment in the commit log for the recommit that says something about what's different this time or why it's okay to recommit.

Noted, thanks!

llvm/unittests/Support/FileCheckTest.cpp

static void expectUndefError(const Twine &ExpectedUndefVarName, Error Err) {
   expectUndefErrors({ExpectedUndefVarName.str()}, std::move(Err));
 }
 
 uint64_t doAdd(uint64_t OpL, uint64_t OpR) { return OpL + OpR; }
 
 TEST_F(FileCheckTest, NumericVariable) {
   // Undefined variable: getValue and eval fail, error returned by eval holds
   // the name of the undefined variable.
-  FileCheckNumericVariable FooVar = FileCheckNumericVariable("FOO", 1);
+  FileCheckNumericVariable FooVar("FOO", 1);
   EXPECT_EQ("FOO", FooVar.getName());
-  FileCheckNumericVariableUse FooVarUse =
-      FileCheckNumericVariableUse("FOO", &FooVar);
+  FileCheckNumericVariableUse FooVarUse("FOO", &FooVar);
   EXPECT_FALSE(FooVar.getValue());
   Expected<uint64_t> EvalResult = FooVarUse.eval();
   ASSERT_FALSE(EvalResult);
   expectUndefError("FOO", EvalResult.takeError());
 
   FooVar.setValue(42);
 
   // Defined variable: getValue and eval return value set.
TEST_F(FileCheckTest, NumericVariable) {
   FooVar.clearValue();
   EXPECT_FALSE(FooVar.getValue());
   EvalResult = FooVarUse.eval();
   ASSERT_FALSE(EvalResult);
   expectUndefError("FOO", EvalResult.takeError());
 }
 
 TEST_F(FileCheckTest, Binop) {
-  FileCheckNumericVariable FooVar = FileCheckNumericVariable("FOO", 1);
+  FileCheckNumericVariable FooVar("FOO", 1);
   FooVar.setValue(42);
   std::unique_ptr<FileCheckNumericVariableUse> FooVarUse =
       std::make_unique<FileCheckNumericVariableUse>("FOO", &FooVar);
-  FileCheckNumericVariable BarVar = FileCheckNumericVariable("BAR", 2);
+  FileCheckNumericVariable BarVar("BAR", 2);
   BarVar.setValue(18);
   std::unique_ptr<FileCheckNumericVariableUse> BarVarUse =
       std::make_unique<FileCheckNumericVariableUse>("BAR", &BarVar);
-  FileCheckASTBinop Binop =
-      FileCheckASTBinop(doAdd, std::move(FooVarUse), std::move(BarVarUse));
+  FileCheckASTBinop Binop(doAdd, std::move(FooVarUse), std::move(BarVarUse));
 
   // Defined variable: eval returns right value.
   Expected<uint64_t> Value = Binop.eval();
   ASSERT_TRUE(bool(Value));
   EXPECT_EQ(60U, *Value);
 
   // 1 undefined variable: eval fails, error contains name of undefined
   // variable.

 }
 
 class PatternTester {
 private:
   size_t LineNumber = 1;
   SourceMgr SM;
   FileCheckRequest Req;
   FileCheckPatternContext Context;
-  FileCheckPattern P =
-      FileCheckPattern(Check::CheckPlain, &Context, LineNumber++);
+  FileCheckPattern P{Check::CheckPlain, &Context, LineNumber++};
 
 public:
   PatternTester() {
     std::vector<std::string> GlobalDefines;
     GlobalDefines.emplace_back(std::string("#FOO=42"));
     GlobalDefines.emplace_back(std::string("BAR=BAZ"));
     // An ASSERT_FALSE would make more sense but cannot be used in a
     // constructor.
TEST_F(FileCheckTest, Substitution) {
   SourceMgr SM;
   FileCheckPatternContext Context;
   std::vector<std::string> GlobalDefines;
   GlobalDefines.emplace_back(std::string("FOO=BAR"));
   EXPECT_FALSE(errorToBool(Context.defineCmdlineVariables(GlobalDefines, SM)));
 
   // Substitution of an undefined string variable fails and error holds that
   // variable's name.
-  FileCheckStringSubstitution StringSubstitution =
-      FileCheckStringSubstitution(&Context, "VAR404", 42);
+  FileCheckStringSubstitution StringSubstitution(&Context, "VAR404", 42);
   Expected<std::string> SubstValue = StringSubstitution.getResult();
   ASSERT_FALSE(bool(SubstValue));
   expectUndefError("VAR404", SubstValue.takeError());
 
   // Substitutions of defined pseudo and non-pseudo numeric variables return
   // the right value.
-  FileCheckNumericVariable LineVar = FileCheckNumericVariable("@LINE", 1);
-  FileCheckNumericVariable NVar = FileCheckNumericVariable("N", 1);
+  FileCheckNumericVariable LineVar("@LINE", 1);
+  FileCheckNumericVariable NVar("N", 1);
   LineVar.setValue(42);
   NVar.setValue(10);
   auto LineVarUse =
       std::make_unique<FileCheckNumericVariableUse>("@LINE", &LineVar);
   auto NVarUse = std::make_unique<FileCheckNumericVariableUse>("N", &NVar);
-  FileCheckNumericSubstitution SubstitutionLine = FileCheckNumericSubstitution(
-      &Context, "@LINE", std::move(LineVarUse), 12);
-  FileCheckNumericSubstitution SubstitutionN =
-      FileCheckNumericSubstitution(&Context, "N", std::move(NVarUse), 30);
+  FileCheckNumericSubstitution SubstitutionLine(&Context, "@LINE",
+                                                std::move(LineVarUse), 12);
+  FileCheckNumericSubstitution SubstitutionN(&Context, "N", std::move(NVarUse),
+                                             30);
   SubstValue = SubstitutionLine.getResult();
   ASSERT_TRUE(bool(SubstValue));
   EXPECT_EQ("42", *SubstValue);
   SubstValue = SubstitutionN.getResult();
   ASSERT_TRUE(bool(SubstValue));
   EXPECT_EQ("10", *SubstValue);
 
   // Substitution of an undefined numeric variable fails, error holds name of
   // undefined variable.
   LineVar.clearValue();
   SubstValue = SubstitutionLine.getResult();
   ASSERT_FALSE(bool(SubstValue));
   expectUndefError("@LINE", SubstValue.takeError());
   NVar.clearValue();
   SubstValue = SubstitutionN.getResult();
   ASSERT_FALSE(bool(SubstValue));
   expectUndefError("N", SubstValue.takeError());
 
   // Substitution of a defined string variable returns the right value.
-  FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Context, 1);
+  FileCheckPattern P(Check::CheckPlain, &Context, 1);
   StringSubstitution = FileCheckStringSubstitution(&Context, "FOO", 42);
   SubstValue = StringSubstitution.getResult();
   ASSERT_TRUE(bool(SubstValue));
   EXPECT_EQ("BAR", *SubstValue);
 }
 
 TEST_F(FileCheckTest, FileCheckContext) {
-  FileCheckPatternContext Cxt = FileCheckPatternContext();
+  FileCheckPatternContext Cxt;
   std::vector<std::string> GlobalDefines;
   SourceMgr SM;
 
   // Missing equal sign.
   GlobalDefines.emplace_back(std::string("LocalVar"));
   EXPECT_TRUE(errorToBool(Cxt.defineCmdlineVariables(GlobalDefines, SM)));
   GlobalDefines.clear();
   GlobalDefines.emplace_back(std::string("#LocalNumVar"));
TEST_F(FileCheckTest, FileCheckContext) {
 
   // Check defined variables are present and undefined is absent.
   StringRef LocalVarStr = "LocalVar";
   StringRef LocalNumVar1Ref = bufferize(SM, "LocalNumVar1");
   StringRef LocalNumVar2Ref = bufferize(SM, "LocalNumVar2");
   StringRef EmptyVarStr = "EmptyVar";
   StringRef UnknownVarStr = "UnknownVar";
   Expected<StringRef> LocalVar = Cxt.getPatternVarValue(LocalVarStr);
-  FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Cxt, 1);
+  FileCheckPattern P(Check::CheckPlain, &Cxt, 1);
   Optional<FileCheckNumericVariable *> DefinedNumericVariable;
   Expected<std::unique_ptr<FileCheckExpressionAST>> ExpressionAST =
       P.parseNumericSubstitutionBlock(LocalNumVar1Ref, DefinedNumericVariable,
                                       /*IsLegacyLineExpr=*/false,
                                       /*LineNumber=*/1, &Cxt, SM);
   ASSERT_TRUE(bool(LocalVar));
   EXPECT_EQ(*LocalVar, "FOO");
   Expected<StringRef> EmptyVar = Cxt.getPatternVarValue(EmptyVarStr);