-
Notifications
You must be signed in to change notification settings - Fork 161
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
Suppress 'Unbound global variable' warning in IsBound #1334
Suppress 'Unbound global variable' warning in IsBound #1334
Conversation
343e7a7
to
4398c7b
Compare
Codecov Report
@@ Coverage Diff @@
## master #1334 +/- ##
==========================================
- Coverage 61.53% 61.48% -0.05%
==========================================
Files 1038 1049 +11
Lines 358256 365574 +7318
Branches 14289 15153 +864
==========================================
+ Hits 220462 224788 +4326
- Misses 134142 136764 +2622
- Partials 3652 4022 +370
|
src/read.c
Outdated
@@ -502,6 +502,7 @@ void ReadCallVarAss ( | |||
WarnOnUnboundGlobalsRNam = RNamName("WarnOnUnboundGlobals"); | |||
|
|||
if ( type == 'g' | |||
&& mode != 'i' | |||
&& STATE(CountNams) != 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe document what these magic values mean.
src/read.c
Outdated
&& (GAPInfo == 0 || !IS_REC(GAPInfo) || !ISB_REC(GAPInfo,WarnOnUnboundGlobalsRNam) || | ||
ELM_REC(GAPInfo,WarnOnUnboundGlobalsRNam) != False ) | ||
&& ! SyCompilePlease ) | ||
if ( type == 'g' /* Reading a global variable */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about using single-line comments // Reading ...
for these new comments? Now that we agreed C99 is fine... :-). Not that this is important in any way
src/read.c
Outdated
&& VAL_GVAR(var) == 0 /* Not an existing global var */ | ||
&& ExprGVar(var) == 0 /* Or an auto var */ | ||
&& ! STATE(IntrIgnoring) /* Not currently ignoring parsed code */ | ||
&& ! GlobalComesFromEnclosingForLoop(var) /* ? */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the /* ? */
? Doesn't seem to be helpful either way. Perhaps: "Don't warn again for index variable of an enclosing for loop"
An example where this matters:
f:=function()
for j in [1..4] do
Print(j,"\n"); od;
end;
If j
is not defined, GAP only warns for the first occurrence of j
. With the GlobalComesFromEnclosingForLoop
call removed, it warns on both. Which kind of makes sense, because by the time you execute the second line, j
will no longer be undefined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The /* ? */
was because I couldn't figure out what that was for. Probably should have cleaned that up before submitting. Thanks for figuring it out.
b89c0f3
to
0860db7
Compare
Tweaked the warning message, as requested by @fingolfin |
At the moment, IsBound(HPCGAP) produces an
Syntax warning: Unbound global variable
warning when used in a function. This is a long-standing issue, but this easy change fixes it.