Check the result of a dynamic_cast.

[drwells]

This was caught by coverity.

[davydden]

i think it should work

if (const auto *data = dynamic_cast<const InternalData *>(&internal_data) )
  {
     ...
  }

[bangerth]

Hm, I find this obfuscating. There is no else branch in the current version of the code, and there shouldn't be one -- the code can only execute if the assertion succeeds.

Is the problem that coverity can't look through what we do in our Assert? Do we need to annotate one of the functions we use there with the attribute [[noreturn]]?

[masterleinad]

As far as I see, coverity is complaining about MappingQ<dim,spacedim>::transform (around line 381) and not about MappingQ<dim,spacedim>::fill_fe_values. Can you double-check?

[drwells]

You're right: 1465505 is indeed for transform. I misread Coverity's report since these blocks are superficially similar.

@bangerth Unfortunately internals::issue_error can return if we do issue_error_nothrow, so this won't work. I think it would be worthwhile to decouple these functions so that a failed Assert really does call a [[noreturn]] function.

[bangerth]

We can't do what you suggest -- we have a bunch of tests that require that we return from a failed Assert via an exception, and that is also a requirement that xSDK has on us.

I don't recall, though -- can a [[noreturn]] function return through an exception? Maybe I misunderstand the code you're referring to.

[masterleinad]

I don't recall, though -- can a [[noreturn]] function return through an exception?

Yes, it is allowed to throw in a [[noreturn]] function but the function mustn't return. This is the first example in http://en.cppreference.com/w/cpp/language/attributes.

[tamiko]

@bangerth The C++ standard draft [1] explicitly lists an example of a no-return function that throws an error. (Stack unwinding in case of an exception is not the same as returning from a call.)

[1] http://eel.is/c++draft/dcl.attr.noreturn

[bangerth]

So could we put that attribute at the right place? When the assertion fails, it calls

    template <class ExceptionType>
    void issue_error (ExceptionHandling  handling,
                      const char       *file,
                      int               line,
                      const char       *function,
                      const char       *cond,
                      const char       *exc_name,
                      ExceptionType     e)
    {
      // Fill the fields of the exception object
      e.set_fields (file, line, function, cond, exc_name);

      switch (handling)
        {
        case abort_on_exception:
          dealii::deal_II_exceptions::internals::abort(e);
          break;
        case abort_nothrow_on_exception:
          // The proper way is to call the function below directly
          // to preserve the noexcept specifier.
          // For reasons of backward compatibility
          // we treat this case here as well.
          issue_error_nothrow(handling, file, line, function, cond, exc_name, e);
          break;
        case throw_on_exception:
          throw e;
        }
    }

which either ends up in internals::abort(), internals::issue_error_nothrow(), or a straight throw.

The first of these functions, internals::abort() either throws, or calls internal_abort() which calls abort(), so we can mark it [[noreturn]]. Patch forthcoming.

The second of these functions, internals::issue_error_nothrow() looks like this:

    void issue_error_nothrow (ExceptionHandling,
                              const char       *file,
                              int               line,
                              const char       *function,
                              const char       *cond,
                              const char       *exc_name,
                              ExceptionBase     e) noexcept
    {
      // Fill the fields of the exception object
      e.set_fields (file, line, function, cond, exc_name);
      if (dealii::deal_II_exceptions::abort_on_exception)
        internal_abort(e);
      else
        {
          // We are not allowed to throw, and not allowed to abort.
          // Just print the exception name to deallog and continue normally:
          deallog << "Exception: " << e.get_exc_name() << std::endl;
          deallog << e.what() << std::endl;
        }
    }

So this function may return, and consequently, issue_error() may return. However, when Assert calls issue_error(), it does so like this:

#define Assert(cond, exc)                                                   \
  {                                                                           \
    if (!(cond))                                                              \
      ::dealii::deal_II_exceptions::internals::                               \
      issue_error(::dealii::deal_II_exceptions::internals::abort_on_exception,\
                  __FILE__, __LINE__, __PRETTY_FUNCTION__, #cond, #exc, exc); \
  }

so when called from Assert, issue_error() always ends up in internals::abort(), which is noreturn. Shouldn't solve this the problem?