1547031 - MOZ_ASSERT doesn't play nice with C++ lambdas

Status: RESOLVED WONTFIX

(Core :: MFBT, defect)

Description

[Julian Seward [:jseward]]

I wanted to write this

  MOZ_ASSERT_IF(!memoryObj,
                 std::all_of(std::begin(dataSegments_),
                             std::end(dataSegments_),
                             [](auto seg) { return !seg->active(); }));

but Clang rejects it for arcane reasons that I don't remotely understand:

  WasmModule.cpp:591:30: error: lambda expression in an unevaluated operand
      [](auto seg) { return !seg->active(); }));
      ^

and Gcc is equally unhelpful:

  WasmModule.cpp:591:30: error: lambda-expression in unevaluated context
      [](auto seg) { return !seg->active(); }));
      ^

I therefore plan-B'd it thusly, on the basis that any half-competent
optimizing compiler would be able to remove "if (!memoryObj) {}" in a
non-debug build:

  if (!memoryObj) {
    MOZ_ASSERT(std::all_of(std::begin(dataSegments_),
                           std::end(dataSegments_),
                           [](auto seg) { return !seg->active(); }));
  }

but no deal there either.  So I have to make do with:

  if (!memoryObj) {
    DebugOnly<bool> allPassive
      = std::all_of(std::begin(dataSegments_), std::end(dataSegments_),
                    [](auto seg) { return !seg->active(); });
    MOZ_ASSERT(allPassive);
  }

which seems a bit feeble.  Can this be fixed?  What I wanted to do doesn't
strike me as unreasonable.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:glandium, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

(In reply to Julian Seward [:jseward] from comment #0)

but Clang rejects it for arcane reasons that I don't remotely understand:

This is just how the C++ language is defined: https://en.cppreference.com/w/cpp/language/lambda

Lambda-expressions are not allowed in unevaluated expressions [until c++20]

I believe in this case the error is because the MOZ_ASSERT macros use the given expression in a decltype(): https://searchfox.org/mozilla-central/source/mfbt/Assertions.h#426
I don't think we'll be able to fix that, so I'm suggesting WONTFIX here. Unless someone knows a good trick?

In the meantime, you'll have to use the DebugOnly workaround you found.
But I would suggest you add #ifdef DEBUG before your if (!memoryObj) test, to emphasize that this whole thing only runs in DEBUG builds.

Comment 3

[Jeff Walden [:Waldo]]

This is a C++ limitation: the type of a lambda is a weird internal-generated thing, and C++ doesn't want it to escape into too many things, and so lambdas can't appear in contexts where they are not actually evaluated into an object of the type that the lambda-expression generates.

The problem is we have a static assertion that the type of the argument to MOZ_ASSERT isn't something vacuously true like a function pointer. The writing out of that, produces one of these unevaluated contexts (a decltype, if memory serves). This assertion in general is probably useful and desirable; it just has this side effect.

I don't know that there's anything we can do about this, except wait for newer C++ to permit it. We could remove the static assertion, but I don't think the benefit for these rare cases outweighs the cost. I've hit this before myself, and #ifdef DEBUG and a helper variable isn't pleasant, but it's not absolutely grotesque.

Will leave open in case froydnj has some special insight, but I don't think this can be anything other than WONTFIX right now.