Opened 9 years ago

Closed 7 years ago

#3235 closed defect (wontfix)

Fix test cases with error messages that include line numbers from the compiler

Reported by: perost Owned by: somebody
Priority: high Milestone: Future
Component: *unknown* Version: trunk
Keywords: Cc: lochel, wbraun, vwaurich

Description (last modified by perost)

I have disabled some test cases that have error messages in them which include line numbers from the compiler. This means that the test cases needs to be updated every time the corresponding source files are changed in the compiler, which is not acceptable. Internal errors should really only be used for things which shouldn't fail, and not used as failtraces.

So either the compiler needs to be fixed so it doesn't print out internal errors for these models, or the models needs to be fixed. The models are:

simulation/modelica/indexreduction/ImplicitStateCoupling.mos
simulation/libraries/3rdParty/MathematicalAspects/01_AlgebraicLoopBoolean.mos
openmodelica/typed-API/SolveLinearSystem.mos
simulation/modelica/algorithms_functions/algorithms.mos
openmodelica/modelicaML/watchdog.mos

Change History (10)

comment:1 Changed 9 years ago by perost

Also, the following pattern is often used in the front end:

function foo
  ...
protected
  Integer err_count = Error.getNumErrorMessages();
algorithm
  try
    // Something which can fail.
  else
    if err_count == Error.getNumErrorMessages() then
      Error.addInternalError(...);
    end if;
  end try;
end foo;

I.e. only print an internal error if the called code fails without printing an error of its own. This is probably a pattern which should be used more in the backend, considering that getNumErrorMessages is used five times in the whole backend, while internal errors are used in ~250 places.

comment:2 follow-up: Changed 9 years ago by vwaurich

Thanks for adressing this issue. I appreciate these recommendation to get a more consistent coding style. Could you add this to the style guide?

comment:3 Changed 9 years ago by sjoelund.se

I will just note that a lot of the addInternalError calls were added by me; they were often replacing print statements (which is worse than just failtrace).

comment:4 in reply to: ↑ 2 Changed 9 years ago by perost

Replying to vwaurich:

Could you add this to the style guide?

Done.

comment:5 Changed 9 years ago by perost

  • Description modified (diff)

algorithms.mos added to the list.

comment:6 Changed 9 years ago by perost

  • Description modified (diff)

Added watchdog.mos too.

comment:7 follow-up: Changed 9 years ago by adrpo

This could be easily fixed: for flag --running-testsuite *do not show the line numbers*
for the errors added with Error.addInternalError.

Last edited 9 years ago by adrpo (previous) (diff)

comment:8 in reply to: ↑ 7 Changed 9 years ago by perost

Replying to adrpo:

This could be easily fixed: for flag --running-testsuite *do not show the line numbers*
for the errors added with Error.addInternalError.

Sure, but that's just a workaround. I don't think we should be using internal errors as failtraces, it causes the actual error messages to be buried beneath all the other errors that are of no consequence to the users.

comment:9 Changed 9 years ago by adrpo

Of course. What I mean is that we could fix the problem until somebody has time to look at all the internal errors and make them proper errors instead.

comment:10 Changed 7 years ago by casella

  • Resolution set to wontfix
  • Status changed from new to closed

It seems like this ticket is totally obsolete

Note: See TracTickets for help on using tickets.