Page MenuHomePhabricator

[clang] Refactor AST printing tests to share more infrastructure
ClosedPublic

Authored by nridge on Jul 5 2021, 10:52 PM.

Diff Detail

Event Timeline

nridge requested review of this revision.Jul 5 2021, 10:52 PM
nridge created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJul 5 2021, 10:52 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

This is the refactor patch discussed in https://reviews.llvm.org/D104619#2846262.

nridge updated this revision to Diff 356605.Jul 5 2021, 11:14 PM

Move a piece of Decl-specific code into DeclPrinterTest.cpp

Looks generally good - few things might be able to be tidied up/simplified, I think?

clang/unittests/AST/ASTPrint.h
61–62

Why is this implemented via specialization but other configuration is implemented via functors (like Printer and PolicyAdjuster) are done via passed in functors? Might be worth using the same mechanism for all of them - this one can have a default argument value in the ctor so it doesn't have to be specified by all uses of this class/they can get this default behavior as implemented here.

clang/unittests/AST/DeclPrinterTest.cpp
53

Is the PrinterType<Decl>{} needed here, or could PrintDecl be passed directly/without explicitly constructing the wrapper? (functions should be implicitly convertible to the lambda type, I think?)

Similarly for all the PrintingPolicyAdjuster(...) - might be able to skip that wrapping expression & rely on an implicit conversion.

clang/unittests/AST/StmtPrinterTest.cpp
55–56

I guess maybe this could be changed (better or worse, not sure) to this:

return PrintedNodeMatches<Stmt>(..., PrintStmt, ...);
nridge updated this revision to Diff 357826.Jul 11 2021, 7:14 PM
nridge marked 3 inline comments as done.

Address review comments

clang/unittests/AST/DeclPrinterTest.cpp
53

Good call on both counts. (The PrintingPolicyAdjuster(...) stuff was left over from when the typedef was for an Optional<function_ref> which you correctly observed was redundant.)

dblaikie accepted this revision.Jul 12 2021, 2:17 PM

Looks reasonable to me, thanks!

This revision is now accepted and ready to land.Jul 12 2021, 2:17 PM
This revision was landed with ongoing or failed builds.Jul 12 2021, 10:48 PM
This revision was automatically updated to reflect the committed changes.
dyung added a subscriber: dyung.Jul 13 2021, 1:18 AM

This change I suspect is causing a build failure on a build bot.

https://lab.llvm.org/buildbot/#/builders/110/builds/4827

FAILED: /usr/bin/c++  -DCLANG_ROUND_TRIP_CC1_ARGS=ON -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/unittests/AST -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/unittests/AST -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/include -Itools/clang/include -Iinclude -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/include -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/utils/unittest/googletest/include -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/utils/unittest/googlemock/include -march=broadwell -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG    -Wno-variadic-macros -fno-exceptions -fno-rtti -UNDEBUG -Wno-suggest-override -std=c++14 -MD -MT tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o -MF tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o.d -o tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o -c /home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/unittests/AST/StmtPrinterTest.cpp
/tmp/cc8Ycpsn.s: Assembler messages:
/tmp/cc8Ycpsn.s:11414: Error: symbol `_ZNSt17_Function_handlerIFbPKN5clang4StmtEENS0_UlS3_E2_EE9_M_invokeERKSt9_Any_dataOS3_' is already defined
/tmp/cc8Ycpsn.s:11937: Error: symbol `_ZNSt14_Function_base13_Base_managerIN5clangUlPKNS1_4StmtEE2_EE10_M_managerERSt9_Any_dataRKS7_St18_Manager_operation' is already defined
ninja: build stopped: subcommand failed.

Can you take a look?

This change I suspect is causing a build failure on a build bot.

https://lab.llvm.org/buildbot/#/builders/110/builds/4827

FAILED: /usr/bin/c++  -DCLANG_ROUND_TRIP_CC1_ARGS=ON -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/unittests/AST -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/unittests/AST -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/include -Itools/clang/include -Iinclude -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/include -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/utils/unittest/googletest/include -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/utils/unittest/googlemock/include -march=broadwell -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG    -Wno-variadic-macros -fno-exceptions -fno-rtti -UNDEBUG -Wno-suggest-override -std=c++14 -MD -MT tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o -MF tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o.d -o tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o -c /home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/unittests/AST/StmtPrinterTest.cpp
/tmp/cc8Ycpsn.s: Assembler messages:
/tmp/cc8Ycpsn.s:11414: Error: symbol `_ZNSt17_Function_handlerIFbPKN5clang4StmtEENS0_UlS3_E2_EE9_M_invokeERKSt9_Any_dataOS3_' is already defined
/tmp/cc8Ycpsn.s:11937: Error: symbol `_ZNSt14_Function_base13_Base_managerIN5clangUlPKNS1_4StmtEE2_EE10_M_managerERSt9_Any_dataRKS7_St18_Manager_operation' is already defined
ninja: build stopped: subcommand failed.

Can you take a look?

Indeed, it also breaks the cross-build for M68k: http://lab.llvm.org:8014/#/builders/180/builds/341

This change I suspect is causing a build failure on a build bot.

https://lab.llvm.org/buildbot/#/builders/110/builds/4827

FAILED: /usr/bin/c++  -DCLANG_ROUND_TRIP_CC1_ARGS=ON -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/unittests/AST -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/unittests/AST -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/include -Itools/clang/include -Iinclude -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/include -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/utils/unittest/googletest/include -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/utils/unittest/googlemock/include -march=broadwell -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG    -Wno-variadic-macros -fno-exceptions -fno-rtti -UNDEBUG -Wno-suggest-override -std=c++14 -MD -MT tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o -MF tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o.d -o tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o -c /home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/unittests/AST/StmtPrinterTest.cpp
/tmp/cc8Ycpsn.s: Assembler messages:
/tmp/cc8Ycpsn.s:11414: Error: symbol `_ZNSt17_Function_handlerIFbPKN5clang4StmtEENS0_UlS3_E2_EE9_M_invokeERKSt9_Any_dataOS3_' is already defined
/tmp/cc8Ycpsn.s:11937: Error: symbol `_ZNSt14_Function_base13_Base_managerIN5clangUlPKNS1_4StmtEE2_EE10_M_managerERSt9_Any_dataRKS7_St18_Manager_operation' is already defined
ninja: build stopped: subcommand failed.

Can you take a look?

Indeed, it also breaks the cross-build for M68k: http://lab.llvm.org:8014/#/builders/180/builds/341

Any ideas what version of the standard library these buildbots are using? This /looks/ a bit like a bug in the standard library in use, perhaps? (also because it's not showing up in lots of other buildbots/developers, I assume?)

To be honest, I don't really understand this error. It seems to come from a compile step (not a link step), and it comes from the assembler and says "symbol ... is already defined"? I don't think I've seen an error like that before. (Typically, errors about duplicate definitions would come from the linker.)

Any suggestions for what it might indicate or how to investigate are welcome.

glaubitz added a comment.EditedJul 13 2021, 12:08 PM

Any ideas what version of the standard library these buildbots are using? This /looks/ a bit like a bug in the standard library in use, perhaps? (also because it's not showing up in lots of other buildbots/developers, I assume?)

I cannot speak for the other builds bots, but the cross-buildbot for m68k which I maintain is running openSUSE Leap 15.3 with GCC 7.5.0.

Maybe this change is compatible with newer versions of GCC only?

Any ideas what version of the standard library these buildbots are using? This /looks/ a bit like a bug in the standard library in use, perhaps? (also because it's not showing up in lots of other buildbots/developers, I assume?)

I cannot speak for the other builds bots, but the cross-buildbot for m68k which I maintain is running openSUSE Leap 15.3 with GCC 7.5.0.

Maybe this change is compatible with newer versions of GCC only?

I'm not sure how to get the standard library version that is being used, but our internal upstream build bot hit this issue, and it is running ubuntu 18.04 with gcc 7.5 as well.

My main dev machine is running ubuntu 20.04 with 9.3 and that does not show the failure, but another one of our internal linux machines with 18.04 and gcc 7.5 does show the failure.

To be honest, I don't really understand this error. It seems to come from a compile step (not a link step), and it comes from the assembler and says "symbol ... is already defined"? I don't think I've seen an error like that before. (Typically, errors about duplicate definitions would come from the linker.)

Any suggestions for what it might indicate or how to investigate are welcome.

Yeah, I'm with you there.

@dyung @glaubitz - perhaps you two could help with a reduced example of what causes this, it does look like a bug in the compiler (yeah, I'd at first assumed a bug in the standard library that caused a link error, but as @nridge pointed out it's not a link error but an assembly error - which really looks like a compiler bug to me) - but likely there's some room for a workaround if we knew what we were dealing with and how to workaround it reliably.

dyung added a comment.EditedJul 13 2021, 11:56 PM

To be honest, I don't really understand this error. It seems to come from a compile step (not a link step), and it comes from the assembler and says "symbol ... is already defined"? I don't think I've seen an error like that before. (Typically, errors about duplicate definitions would come from the linker.)

Any suggestions for what it might indicate or how to investigate are welcome.

Yeah, I'm with you there.

@dyung @glaubitz - perhaps you two could help with a reduced example of what causes this, it does look like a bug in the compiler (yeah, I'd at first assumed a bug in the standard library that caused a link error, but as @nridge pointed out it's not a link error but an assembly error - which really looks like a compiler bug to me) - but likely there's some room for a workaround if we knew what we were dealing with and how to workaround it reliably.

Sorry, just realized you did not do the revert.

dyung added a comment.EditedJul 14 2021, 1:39 AM

If it helps, I have so far been able to reduce the file to this which still shows the failure when compiled with gcc 7.5:

#include "ASTPrint.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/SmallString.h"
#include "gtest/gtest.h"

using namespace clang;
using namespace ast_matchers;
//using namespace tooling;                                                                                                                                                                                                                                         

namespace {

enum class StdVer { CXX98, CXX11, CXX14, CXX17, CXX2a };

DeclarationMatcher FunctionBodyMatcher(StringRef ContainingFunction) {
  return functionDecl(hasName(ContainingFunction),
                      has(compoundStmt(has(stmt().bind("id")))));
}

static void PrintStmt(raw_ostream &Out, const ASTContext *Context,
                      const Stmt *S, PrintingPolicyAdjuster PolicyAdjuster) {
  assert(S != nullptr && "Expected non-null Stmt");
  PrintingPolicy Policy = Context->getPrintingPolicy();
  if (PolicyAdjuster)
    PolicyAdjuster(Policy);
  S->printPretty(Out, /*Helper*/ nullptr, Policy);
}

template <typename Matcher>
::testing::AssertionResult
PrintedStmtMatches(StringRef Code, const std::vector<std::string> &Args,
                   const Matcher &NodeMatch, StringRef ExpectedPrinted,
                   PrintingPolicyAdjuster PolicyAdjuster = nullptr) {
  return PrintedNodeMatches<Stmt>(Code, Args, NodeMatch, ExpectedPrinted, "",
                                  PrintStmt, PolicyAdjuster);
}

template <typename T>
::testing::AssertionResult
PrintedStmtCXXMatches(StdVer Standard, StringRef Code, const T &NodeMatch,
                      StringRef ExpectedPrinted,
                      PrintingPolicyAdjuster PolicyAdjuster = nullptr) {
  const char *StdOpt;
  switch (Standard) {
  case StdVer::CXX98: StdOpt = "-std=c++98"; break;
  case StdVer::CXX11: StdOpt = "-std=c++11"; break;
  case StdVer::CXX14: StdOpt = "-std=c++14"; break;
  case StdVer::CXX17: StdOpt = "-std=c++17"; break;
  case StdVer::CXX2a: StdOpt = "-std=c++2a"; break;
  }

  std::vector<std::string> Args = {
    StdOpt,
    "-Wno-unused-value",
  };
  return PrintedStmtMatches(Code, Args, NodeMatch, ExpectedPrinted,
                            PolicyAdjuster);
}

} // unnamed namespace

TEST(StmtPrinter, TestIntegerLiteral) {
  ASSERT_TRUE(PrintedStmtCXXMatches(StdVer::CXX98,
    "void A() {"
    "  1, -1, 1U, 1u,"
    "  1L, 1l, -1L, 1UL, 1ul,"
    "  1LL, -1LL, 1ULL;"
    "}",
    FunctionBodyMatcher("A"),
    "1 , -1 , 1U , 1U , "
    "1L , 1L , -1L , 1UL , 1UL , "
    "1LL , -1LL , 1ULL"));
    // Should be: with semicolon                                                                                                                                                                                                                                   
}

TEST(StmtPrinter, TestCXXConversionDeclImplicit) {
  ASSERT_TRUE(PrintedStmtCXXMatches(
      StdVer::CXX98,
      "struct A {"
      "operator void *();"
      "A operator&(A);"
      "};"
      "void bar(void *);"
      "void foo(A a, A b) {"
      "  bar(a & b);"
      "}",
      traverse(TK_AsIs, cxxMemberCallExpr(anything()).bind("id")), "a & b"));
}

When compiled with gcc 7.5, it produces the attached (gzip'd) assembly file which is what is what is giving the errors:

GNU assembler version 2.30 (x86_64-linux-gnu) using BFD version (GNU Binutils for Ubuntu) 2.30
StmtPrinterTest2.s: Assembler messages:
StmtPrinterTest2.s:928: Error: symbol `_ZNSt14_Function_base13_Base_managerIN5clangUlPKNS1_4StmtEE2_EE10_M_managerERSt9_Any_dataRKS7_St18_Manager_operation' is already defined
StmtPrinterTest2.s:9924: Error: symbol `_ZNSt17_Function_handlerIFbPKN5clang4StmtEENS0_UlS3_E2_EE9_M_invokeERKSt9_Any_dataOS3_' is already defined

If it helps, I have so far been able to reduce the file to this which still shows the failure when compiled with gcc 7.5:

Could you provide the preprocessed input file - maybe that run through creduce could be helpful.

dyung added a comment.Jul 14 2021, 1:33 PM

If it helps, I have so far been able to reduce the file to this which still shows the failure when compiled with gcc 7.5:

Could you provide the preprocessed input file - maybe that run through creduce could be helpful.

I did originally run it through cvise, but wasn't sure if it would be useful since it cut out everything except for what was causing one of the two failures. But if it helps, here is what it reduced to:

namespace std {
template <bool> using __bool_constant = int;
template <typename> struct is_base_of;
template <typename> using remove_reference_t = int;
template <bool> using enable_if_t = int;
template <typename> struct iterator_traits;
struct _Any_data;
enum _Manager_operation {};
template <typename> class function;
class _Function_base {
public:
  template <typename> class _Base_manager {
  public:
    static bool _M_manager(_Any_data &, const _Any_data &, _Manager_operation) {
    }
  };
  typedef bool (*_Manager_type)(_Any_data &, const _Any_data &,
                                _Manager_operation);
  _Manager_type _M_manager;
};
template <typename, typename> class _Function_handler;
template <typename _Res, typename _Functor, typename... _ArgTypes>
class _Function_handler<_Res(_ArgTypes...), _Functor>
    : public _Function_base::_Base_manager<_Functor> {};
template <typename _Res, typename... _ArgTypes>
class function<_Res(_ArgTypes...)> : _Function_base {
  template <typename, typename> using _Requires = int;
public:
  template <typename _Functor,
            typename = _Requires<__bool_constant<!bool()>, void>,
            typename = _Requires<int, void>>
  function(_Functor);
};
template <typename _Res, typename... _ArgTypes>
template <typename _Functor, typename, typename>
function<_Res(_ArgTypes...)>::function(_Functor) {
  _M_manager = _Function_handler<_Res(), _Functor>::_M_manager;
}
} // namespace std
template <typename IterTy>
void hasNItems(
    IterTy Begin = [] {},
    std::enable_if_t<std::is_base_of<std::iterator_traits<
        std::remove_reference_t<decltype(Begin)>>>::valuevoid> = [] {},
    std::enable_if_t<std::is_base_of<std::iterator_traits<
        std::remove_reference_t<decltype(Begin)>>>::valuevoid> = [] {});
class StringRef {
public:
  StringRef(char *);
};
namespace clang {
class Stmt anything();
template <typename> using NodeFilter = std::function<bool()>;
template <typename NodeType, typename Matcher>
void PrintedNodeMatches(
    Matcher, NodeFilter<NodeType> = [](const NodeType *) {});
} // namespace clang
using namespace clang;
enum StdVer { CXX98 };
template <typename Matcher> void PrintedStmtMatches(Matcher NodeMatch) {
  PrintedNodeMatches<Stmt>(NodeMatch);
}
template <typename T>
void PrintedStmtCXXMatches(StdVer, StringRef, T NodeMatch, StringRef) {
  PrintedStmtMatches(NodeMatch);
  PrintedStmtCXXMatches(CXX98, "", 0, "");
}
void StmtPrinter_TestCXXConversionDeclImplicit_TestTestBody() {
  PrintedStmtCXXMatches(CXX98, "", anything, "");
}

Compiled with gcc 7.5 and optimizations yields the following output:

dyung@frogstar:~/sandbox/StmtPrinterTest$ gcc -O2 -Wno-write-strings StmtPrinterTest.preproc.cpp
/tmp/ccLNKEkE.s: Assembler messages:
/tmp/ccLNKEkE.s:14: Error: symbol `_ZNSt14_Function_base13_Base_managerIN5clangUlPKNS1_4StmtEE2_EE10_M_managerERSt9_Any_dataRKS7_St18_Manager_operation' is already defined
dyung added a comment.Jul 14 2021, 1:41 PM

In case it is useful, here is the full preprocessed file.

Thanks! I reduced it further to:

template <typename>
class function {
public:
  template <typename F>
  function(F) {}
};

template <typename M>
void Foo(M, function<void()> = [](){});

void Bar() {
  Foo(42);
  Foo(42.0);
}

with gcc 7, this gives:

/tmp/cccKjL8O.s: Assembler messages:
/tmp/cccKjL8O.s:76: Error: symbol `_ZN8functionIFvvEEC2IUlvE_EET_' is already defined

I still don't understand this error, but it seems to be related to the lambda that appears as a default argument in a function that gets instantiated multiple times (and with the default argument value not dependent on the function's template parameter), so I think that gives us some idea of where to try workarounds.

Making the default argument a non-lambda seems to be sufficient to avoid the error:

template <typename>
class function {
public:
  template <typename F>
  function(F) {}
};

void DefaultFunc();

template <typename M>
void Foo(M, function<void()> = DefaultFunc);

void Bar() {
  Foo(42);
  Foo(42.0);
}
dyung added a comment.Jul 14 2021, 2:06 PM

Making the default argument a non-lambda seems to be sufficient to avoid the error:

template <typename>
class function {
public:
  template <typename F>
  function(F) {}
};

void DefaultFunc();

template <typename M>
void Foo(M, function<void()> = DefaultFunc);

void Bar() {
  Foo(42);
  Foo(42.0);
}

Interesting. If you can get me an updated patch, I can give it a try on my machine with 7.5 to verify if you like.

nridge reopened this revision.Jul 14 2021, 2:15 PM
This revision is now accepted and ready to land.Jul 14 2021, 2:15 PM
nridge updated this revision to Diff 358750.Jul 14 2021, 2:15 PM

Work around a gcc 7 bug

Interesting. If you can get me an updated patch, I can give it a try on my machine with 7.5 to verify if you like.

I posted an updated patch, if you can give that a whirl that would be great!

dyung accepted this revision.Jul 14 2021, 3:53 PM

Interesting. If you can get me an updated patch, I can give it a try on my machine with 7.5 to verify if you like.

I posted an updated patch, if you can give that a whirl that would be great!

I can confirm that it builds and tests successfully on a machine with gcc 7.5. Assuming you have tested with more recent compilers and that it is fine there, LGTM.

This revision was landed with ongoing or failed builds.Jul 14 2021, 4:44 PM
This revision was automatically updated to reflect the committed changes.

@dyung I appreciate the help tracking this down!