Figure out a way to mark things as unreachable by unit tests

[olabini]

https://trac.torproject.org/projects/tor/ticket/16792

[tdruiva]

   Exclusion markers

   To exclude specific lines of code from a tracefile, you can add  exclu-
   sion  markers to the source code. Additionally you can exclude specific
   branches from branch coverage without excluding the involved lines from
   line  and  function  coverage. Exclusion markers are keywords which can
   for example be added in the form of a comment.

   The following markers are recognized by geninfo:

   LCOV_EXCL_LINE
          Lines containing this marker will be excluded.
   LCOV_EXCL_START
          Marks the beginning of an excluded section. The current line  is
          part of this section.
   LCOV_EXCL_STOP
          Marks  the end of an excluded section. The current line not part
          of this section.
   LCOV_EXCL_BR_LINE
          Lines containing this marker will be excluded from branch cover-
          age.
   LCOV_EXCL_BR_START
          Marks  the  beginning of a section which is excluded from branch
          coverage. The current line is part of this section.
   LCOV_EXCL_BR_STOP
          Marks the end of a section which is excluded from branch  cover-
          age. The current line not part of this section.

[juniorz]

\o/

[juniorz]

The current code makes me believe it would be expected to have this in a macro, rather than a comment.

I would expect something along the lines of:

switch(some_value){
  case 1:
    // something
    break;
  case 0:
    // something else
    break;
  case -1:
    TOR_COV_UNREACHABLE_BEGIN
    //unreachable code
    TOR_COV_UNREACHABLE_ENF
}

but we have to double check with the Tor team.

I assume the macro would expand to LCOV_EXCL_START and LCOV_EXCL_STOP only if coverage is enable.

Who wants to start the conversation? ;)

[tdruiva]

Good point.
I'm starting a conversation by ticket message.

[tdruiva]

https://trac.torproject.org/projects/tor/ticket/16792#comment:3

[olabini]

Reinaldo, not sure I understand your point. What is the reason for having this be in macro's instead of in a comment?

[olabini]

Specifically, I think using macros for this is a bad idea, since macros usually have functional impliciations, while these things are specifically non-functional annotations.

[juniorz]

My reasoning is to abstract the tool we are using.

Tor source code names their existing macros ad COVERAGE_ENABLED (and
TOR_COVERAGE) rather than "LCOV_ENABLED". On the same line, having
/LCOV_EXCL_START/ inside the code seems to be wrong, according to the
existing naming standard.

For this reason, I suggested to double check with the community before
assuming LCOV_EXCL_START is the way to go.

[olabini]

OK, I accept that reasoning. =)

[olabini]

however. that depends on whether LCOV expands macros or not when looking for those names

[tcz001]

Put it into macro could be a place where the coverage tool never goes to, I would prefer a trial before we say it's a solution or not.

[juniorz]

I think my "proposal" is not feasible. "The C preprocessor converts comments to whitespace before macros are even considered"[1]

1 - https://gcc.gnu.org/onlinedocs/cpp/Concatenation.html