Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Teuchos: TEUCHOS_FUNC_TIME_MONITOR_DIFF with -Werror=shadow #12358

Closed
kliegeois opened this issue Oct 3, 2023 · 3 comments
Closed

Teuchos: TEUCHOS_FUNC_TIME_MONITOR_DIFF with -Werror=shadow #12358

kliegeois opened this issue Oct 3, 2023 · 3 comments
Labels
pkg: Teuchos Issues primarily dealing with the Teuchos Package type: bug The primary issue is a bug in Trilinos code or tests

Comments

@kliegeois
Copy link
Contributor

Bug Report

@bartlettroscoe
@trilinos/teuchos

Description

When building the branch of #12344 with:

-D   Ifpack2_ENABLE_BlockTriDiContainer_Timers:BOOL=ON \

and the C++ flag:

-Werror=shadow

TEUCHOS_FUNC_TIME_MONITOR_DIFF does not compile because of the nesting of TEUCHOS_FUNC_TIME_MONITOR. Because, the lines https://github.com/trilinos/Trilinos/blob/master/packages/teuchos/comm/src/Teuchos_TimeMonitor.hpp#L91-L98 throw the following error:

error: declaration of 'mainblabla_localTimeMonitor' shadows a previous local

@bartlettroscoe could we use FUNCNAME in DIFF ## blabla_localTimer (I am not a macro expert)?

@kliegeois kliegeois added type: bug The primary issue is a bug in Trilinos code or tests pkg: Teuchos Issues primarily dealing with the Teuchos Package labels Oct 3, 2023
@bartlettroscoe
Copy link
Member

@kliegeois, can you please copy and paste the entire build error output so that we can see the full error context? One way to do this is to just run:

$ make dashboard

and then the build error will show up on CDash (the STDOUT will give you the exact URL to the build on CDash at the very end of running that command).

@kliegeois
Copy link
Contributor Author

@bartlettroscoe thanks for your comment!

Please find attached the asked file:
log.txt

@bartlettroscoe
Copy link
Member

@kliegeois, sorry to only to be getting to this now (going back through my email backlog) ...

Is this still an issue? It looks like the PR:

has not been merged yet.

I see the problem is that there are two calls to IFPACK2_BLOCKHELPER_TIMER() in one function:

IFPACK2_BLOCKHELPER_TIMER("createPartInterface");

and

IFPACK2_BLOCKHELPER_TIMER("determine part Jacobi");

That is causing a shadowing warning.

One suggestion to get around this, why not just break off that code that calls the timer IFPACK2_BLOCKHELPER_TIMER("determine part Jacobi") into its own function that is called by createPartInterface(...)? That would eliminate this shadow warning and might also make this code easier to follow. (As a rule of thumb, it is nice if a function implementation can fit in a single window without having to scroll.)

That function is very long starting at:

IFPACK2_BLOCKHELPER_TIMER("createPartInterface");

and going all the way down to:

with lots of detail.

@bartlettroscoe could we use FUNCNAME in DIFF ## blabla_localTimer (I am not a macro expert)?

I don't think you can use FUNCNAME because it can contain chars that are not allowed in a macro name, like : or (.

The C/C++ preprocessor language is pretty limited in the type of string manipulation you can do at pre-processing time. For example, see:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: Teuchos Issues primarily dealing with the Teuchos Package type: bug The primary issue is a bug in Trilinos code or tests
Projects
None yet
Development

No branches or pull requests

2 participants