Fix ifdef DEBUGs to NDEBUG #5175

ZUUL42 · 2019-05-14T23:13:10Z

@trilinos/triutils

Description

Correcting ifdef DEBUG to NDEBUG for recent changes in #5153.

Checklist

My commit messages mention the appropriate GitHub issue numbers.
My code follows the code style of the affected package(s).
My change requires a change to the documentation.
I have read the code contribution guidelines for this project.
All new and existing tests passed.
No new compiler warnings were introduced.
These changes break backwards compatibility.

mhoemmen

Thanks, but it's actually #ifndef NDEBUG

mhoemmen · 2019-05-14T23:16:38Z

packages/triutils/src/Trilinos_Util_ReadHpc2Epetra.cpp

@@ -80,7 +80,7 @@ void Trilinos_Util_ReadHpc2Epetra_internal(
    exit(1);
  }
  int_type numGlobalEquations, total_nnz;
-#ifdef DEBUG
+#ifdef NDEBUG


It's ifndef NDEBUG. assert only exists if NDEBUG is not defined.

https://en.cppreference.com/w/c/error/assert

Ahh! Thanks.
I'm trying to burn through this a bit too fast today.

trilinos-autotester · 2019-05-14T23:38:36Z

Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request.

trilinos-autotester · 2019-05-14T23:41:39Z

Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects:

Pull Request Auto Testing STARTING (click to expand)

Build Information

Test Name: Trilinos_pullrequest_gcc_4.8.4

Build Num: 3565
Status: STARTED

Jenkins Parameters

Parameter Name	Value
COMPILER_MODULE	sems-gcc/4.8.4
JENKINS_BUILD_TYPE	Release
JENKINS_COMM_TYPE	MPI
JENKINS_DO_COMPLEX	OFF
JENKINS_JOB_TYPE	Experimental
MPI_MODULE	sems-openmpi/1.8.7
PULLREQUESTNUM	5175
TEST_REPO_ALIAS	TRILINOS
TRILINOS_SOURCE_BRANCH	Werror_Triutils
TRILINOS_SOURCE_REPO	https://github.com/ZUUL42/Trilinos
TRILINOS_SOURCE_SHA	`eb2ed98`
TRILINOS_TARGET_BRANCH	develop
TRILINOS_TARGET_REPO	https://github.com/trilinos/Trilinos
TRILINOS_TARGET_SHA	`6df3437`

Build Information

Test Name: Trilinos_pullrequest_intel_17.0.1

Build Num: 3392
Status: STARTED

Jenkins Parameters

Parameter Name	Value
PULLREQUESTNUM	5175
TEST_REPO_ALIAS	TRILINOS
TRILINOS_SOURCE_BRANCH	Werror_Triutils
TRILINOS_SOURCE_REPO	https://github.com/ZUUL42/Trilinos
TRILINOS_SOURCE_SHA	`eb2ed98`
TRILINOS_TARGET_BRANCH	develop
TRILINOS_TARGET_REPO	https://github.com/trilinos/Trilinos
TRILINOS_TARGET_SHA	`6df3437`

Build Information

Test Name: Trilinos_pullrequest_gcc_4.9.3_SERIAL

Build Num: 1837
Status: STARTED

Jenkins Parameters

Parameter Name	Value
PULLREQUESTNUM	5175
TEST_REPO_ALIAS	TRILINOS
TRILINOS_SOURCE_BRANCH	Werror_Triutils
TRILINOS_SOURCE_REPO	https://github.com/ZUUL42/Trilinos
TRILINOS_SOURCE_SHA	`eb2ed98`
TRILINOS_TARGET_BRANCH	develop
TRILINOS_TARGET_REPO	https://github.com/trilinos/Trilinos
TRILINOS_TARGET_SHA	`6df3437`

Build Information

Test Name: Trilinos_pullrequest_gcc_7.2.0

Build Num: 1598
Status: STARTED

Jenkins Parameters

Parameter Name	Value
PULLREQUESTNUM	5175
TEST_REPO_ALIAS	TRILINOS
TRILINOS_SOURCE_BRANCH	Werror_Triutils
TRILINOS_SOURCE_REPO	https://github.com/ZUUL42/Trilinos
TRILINOS_SOURCE_SHA	`eb2ed98`
TRILINOS_TARGET_BRANCH	develop
TRILINOS_TARGET_REPO	https://github.com/trilinos/Trilinos
TRILINOS_TARGET_SHA	`6df3437`

Build Information

Test Name: Trilinos_pullrequest_cuda_9.2

Build Num: 1243
Status: STARTED

Jenkins Parameters

Parameter Name	Value
JENKINS_JOB_TYPE	Experimental
PULLREQUESTNUM	5175
TEST_REPO_ALIAS	TRILINOS
TRILINOS_SOURCE_BRANCH	Werror_Triutils
TRILINOS_SOURCE_REPO	https://github.com/ZUUL42/Trilinos
TRILINOS_SOURCE_SHA	`eb2ed98`
TRILINOS_TARGET_BRANCH	develop
TRILINOS_TARGET_REPO	https://github.com/trilinos/Trilinos
TRILINOS_TARGET_SHA	`6df3437`

Using Repos:

Repo: TRILINOS (ZUUL42/Trilinos)

Pull Request Author: ZUUL42

trilinos-autotester · 2019-05-15T03:17:02Z

Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED

Pull Request Auto Testing has PASSED (click to expand)

Build Information

Test Name: Trilinos_pullrequest_gcc_4.8.4

Build Num: 3565
Status: PASSED

Jenkins Parameters

Parameter Name	Value
COMPILER_MODULE	sems-gcc/4.8.4
JENKINS_BUILD_TYPE	Release
JENKINS_COMM_TYPE	MPI
JENKINS_DO_COMPLEX	OFF
JENKINS_JOB_TYPE	Experimental
MPI_MODULE	sems-openmpi/1.8.7
PULLREQUESTNUM	5175
TEST_REPO_ALIAS	TRILINOS
TRILINOS_SOURCE_BRANCH	Werror_Triutils
TRILINOS_SOURCE_REPO	https://github.com/ZUUL42/Trilinos
TRILINOS_SOURCE_SHA	`eb2ed98`
TRILINOS_TARGET_BRANCH	develop
TRILINOS_TARGET_REPO	https://github.com/trilinos/Trilinos
TRILINOS_TARGET_SHA	`6df3437`

Build Information

Test Name: Trilinos_pullrequest_intel_17.0.1

Build Num: 3392
Status: PASSED

Jenkins Parameters

Parameter Name	Value
PULLREQUESTNUM	5175
TEST_REPO_ALIAS	TRILINOS
TRILINOS_SOURCE_BRANCH	Werror_Triutils
TRILINOS_SOURCE_REPO	https://github.com/ZUUL42/Trilinos
TRILINOS_SOURCE_SHA	`eb2ed98`
TRILINOS_TARGET_BRANCH	develop
TRILINOS_TARGET_REPO	https://github.com/trilinos/Trilinos
TRILINOS_TARGET_SHA	`6df3437`

Build Information

Test Name: Trilinos_pullrequest_gcc_4.9.3_SERIAL

Build Num: 1837
Status: PASSED

Jenkins Parameters

Parameter Name	Value
PULLREQUESTNUM	5175
TEST_REPO_ALIAS	TRILINOS
TRILINOS_SOURCE_BRANCH	Werror_Triutils
TRILINOS_SOURCE_REPO	https://github.com/ZUUL42/Trilinos
TRILINOS_SOURCE_SHA	`eb2ed98`
TRILINOS_TARGET_BRANCH	develop
TRILINOS_TARGET_REPO	https://github.com/trilinos/Trilinos
TRILINOS_TARGET_SHA	`6df3437`

Build Information

Test Name: Trilinos_pullrequest_gcc_7.2.0

Build Num: 1598
Status: PASSED

Jenkins Parameters

Parameter Name	Value
PULLREQUESTNUM	5175
TEST_REPO_ALIAS	TRILINOS
TRILINOS_SOURCE_BRANCH	Werror_Triutils
TRILINOS_SOURCE_REPO	https://github.com/ZUUL42/Trilinos
TRILINOS_SOURCE_SHA	`eb2ed98`
TRILINOS_TARGET_BRANCH	develop
TRILINOS_TARGET_REPO	https://github.com/trilinos/Trilinos
TRILINOS_TARGET_SHA	`6df3437`

Build Information

Test Name: Trilinos_pullrequest_cuda_9.2

Build Num: 1243
Status: PASSED

Jenkins Parameters

Parameter Name	Value
JENKINS_JOB_TYPE	Experimental
PULLREQUESTNUM	5175
TEST_REPO_ALIAS	TRILINOS
TRILINOS_SOURCE_BRANCH	Werror_Triutils
TRILINOS_SOURCE_REPO	https://github.com/ZUUL42/Trilinos
TRILINOS_SOURCE_SHA	`eb2ed98`
TRILINOS_TARGET_BRANCH	develop
TRILINOS_TARGET_REPO	https://github.com/trilinos/Trilinos
TRILINOS_TARGET_SHA	`6df3437`

CDash Test Results for PR# 5175.

trilinos-autotester · 2019-05-15T03:17:20Z

Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
THE LAST COMMIT TO THIS PULL REQUEST HAS NOT BEEN REVIEWED YET!

trilinos-autotester · 2019-05-15T03:17:27Z

All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur...

bathmatt · 2019-05-15T13:34:24Z

Let me know via my SNL email what other patches you need reviewed, you mentioned yesterday that there were other MRs ??

jmgate · 2019-05-15T14:11:47Z

packages/triutils/src/Trilinos_Util_ReadHpc2Epetra.cpp

@@ -91,12 +91,12 @@ void Trilinos_Util_ReadHpc2Epetra_internal(
  // the warning is easy to fix.
  if(sizeof(int) == sizeof(int_type)) {
    int numGlobalEquations_int, total_nnz_int;
-#ifdef DEBUG
+#ifndef NDEBUG
    cnt =
 #endif
    fscanf(in_file,"%d",&numGlobalEquations_int);
    assert(cnt > 0);


Doesn't this assert() line need to be inside one of the #if blocks?

@jmgate It's fine -- assert has the following definition, according to https://en.cppreference.com/w/cpp/error/assert :

#ifdef NDEBUG #define assert(condition) ((void)0) #else #define assert(condition) /*implementation defined*/ #endif

The macro assert() is only defined if the macro NDEBUG is not defined.

Does the fact that all other occurrences of cnt are wrapped in an #if not come into play here? @mhoemmen, it sounds like you're saying line 98 gets replaced with ((void)0) so it won't throw this error again?

error: ‘cnt’ was not declared in this scope

I'm asking Santa for a Trilinos without #ifdefs this year.

@jmgate wrote:

@mhoemmen, it sounds like you're saying line 98 gets replaced with ((void)0) so it won't throw this error again?

Yes, that's correct. assert is a macro. If NDEBUG is defined, then the macro's argument does not appear in the compiled source code. That is, in fact, why the compiler was warning about "unused variables."

I'm asking Santa for a Trilinos without #ifdefs this year.

I ... don't have anything constructive to say about this ;-)

mhoemmen

This looks fixed.

mhoemmen · 2019-05-15T16:02:42Z

packages/triutils/src/Trilinos_Util_ReadHpc2Epetra.cpp

@@ -91,12 +91,12 @@ void Trilinos_Util_ReadHpc2Epetra_internal(
  // the warning is easy to fix.
  if(sizeof(int) == sizeof(int_type)) {
    int numGlobalEquations_int, total_nnz_int;
-#ifdef DEBUG
+#ifndef NDEBUG
    cnt =
 #endif
    fscanf(in_file,"%d",&numGlobalEquations_int);
    assert(cnt > 0);


@jmgate It's fine -- assert has the following definition, according to https://en.cppreference.com/w/cpp/error/assert :

#ifdef NDEBUG #define assert(condition) ((void)0) #else #define assert(condition) /*implementation defined*/ #endif

bartlettroscoe · 2019-05-15T16:11:58Z

We need to make one of the Trilinos PR builds set CMAKE_BUILD_TYPE=DEBUG which will not define NDEBUG and will catch this type of error in the future. See #5184.

ZUUL42 · 2019-05-15T16:18:52Z

We need to make one of the Trilinos PR builds set CMAKE_BUILD_TYPE=DEBUG which will not define NDEBUG and will catch this type of error in the future. See #5184.

We were just discussing the possibility and path forward for a PR debug build at the standup this morning. Nothing concrete yet.

trilinos-autotester · 2019-05-15T16:42:27Z

Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
THE LAST COMMIT TO THIS PULL REQUEST HAS BEEN REVIEWED, BUT NOT ACCEPTED OR REQUIRES CHANGES

trilinos-autotester · 2019-05-15T16:42:35Z

All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur...

@mhoemmen

@mhoemmen says all is well.

mhoemmen

Approved, provided that @ZUUL42 tested with a debug build and that the debug build builds this package warning free.

trilinos-autotester · 2019-05-15T18:34:25Z

Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ mhoemmen bathmatt ]!

trilinos-autotester · 2019-05-15T18:34:34Z

Status Flag 'Pull Request AutoTester' - Pull Request will be Automerged

trilinos-autotester · 2019-05-15T18:34:40Z

Merge on Pull Request# 5175: IS A SUCCESS - Pull Request successfully merged

ZUUL42 added pkg: TriUtils AT: AUTOMERGE Causes the PR autotester to automatically merge the PR branch once approvals are completed labels May 14, 2019

ZUUL42 self-assigned this May 14, 2019

mhoemmen requested changes May 14, 2019

View reviewed changes

Correct ifdef DEBUG to ifndef NDEBUG

eb2ed98

ZUUL42 force-pushed the Werror_Triutils branch from 29470ae to eb2ed98 Compare May 14, 2019 23:30

ZUUL42 requested a review from bathmatt May 14, 2019 23:55

bathmatt approved these changes May 15, 2019

View reviewed changes

jmgate previously requested changes May 15, 2019

View reviewed changes

mhoemmen approved these changes May 15, 2019

View reviewed changes

ZUUL42 mentioned this pull request May 15, 2019

Fix warnings and set Werror for Triutils in GCC 7.2.0 build #5153

Merged

7 tasks

mhoemmen approved these changes May 15, 2019

View reviewed changes

trilinos-autotester merged commit 6b7c754 into trilinos:develop May 15, 2019

trilinos-autotester removed the AT: AUTOMERGE Causes the PR autotester to automatically merge the PR branch once approvals are completed label May 15, 2019

bartlettroscoe added ATDM Sev: Critical Problems that critically damage ability to run ATDM Trilinos builds much less allow APP updates client: ATDM Any issue primarily impacting the ATDM project labels May 15, 2019

bartlettroscoe added type: bug The primary issue is a bug in Trilinos code or tests PA: Data Services Issues that fall under the Trilinos Data Services Product Area labels May 15, 2019

trilinos-autotester mentioned this pull request Apr 6, 2022

Seacas script syntax fix #10411

Merged

Fix ifdef DEBUGs to NDEBUG #5175

Fix ifdef DEBUGs to NDEBUG #5175

Conversation

ZUUL42 commented May 14, 2019

Description

Checklist

mhoemmen left a comment

Choose a reason for hiding this comment

mhoemmen May 14, 2019

Choose a reason for hiding this comment

ZUUL42 May 14, 2019 • edited Loading

Choose a reason for hiding this comment

trilinos-autotester commented May 14, 2019

trilinos-autotester commented May 14, 2019

Build Information

Test Name: Trilinos_pullrequest_gcc_4.8.4

Jenkins Parameters

Build Information

Test Name: Trilinos_pullrequest_intel_17.0.1

Jenkins Parameters

Build Information

Test Name: Trilinos_pullrequest_gcc_4.9.3_SERIAL

Jenkins Parameters

Build Information

Test Name: Trilinos_pullrequest_gcc_7.2.0

Jenkins Parameters

Build Information

Test Name: Trilinos_pullrequest_cuda_9.2

Jenkins Parameters

Using Repos:

trilinos-autotester commented May 15, 2019

Build Information

Test Name: Trilinos_pullrequest_gcc_4.8.4

Jenkins Parameters

Build Information

Test Name: Trilinos_pullrequest_intel_17.0.1

Jenkins Parameters

Build Information

Test Name: Trilinos_pullrequest_gcc_4.9.3_SERIAL

Jenkins Parameters

Build Information

Test Name: Trilinos_pullrequest_gcc_7.2.0

Jenkins Parameters

Build Information

Test Name: Trilinos_pullrequest_cuda_9.2

Jenkins Parameters

trilinos-autotester commented May 15, 2019

trilinos-autotester commented May 15, 2019

bathmatt commented May 15, 2019

jmgate May 15, 2019 • edited Loading

Choose a reason for hiding this comment

mhoemmen May 15, 2019

Choose a reason for hiding this comment

bartlettroscoe May 15, 2019

Choose a reason for hiding this comment

jmgate May 15, 2019

Choose a reason for hiding this comment

mhoemmen May 15, 2019

Choose a reason for hiding this comment

mhoemmen left a comment

Choose a reason for hiding this comment

mhoemmen May 15, 2019

Choose a reason for hiding this comment

bartlettroscoe commented May 15, 2019

ZUUL42 commented May 15, 2019

trilinos-autotester commented May 15, 2019

trilinos-autotester commented May 15, 2019

mhoemmen left a comment

Choose a reason for hiding this comment

trilinos-autotester commented May 15, 2019

trilinos-autotester commented May 15, 2019

trilinos-autotester commented May 15, 2019

ZUUL42 May 14, 2019 •

edited

Loading

jmgate May 15, 2019 •

edited

Loading