Add option to only warn about macro redefinitions that change the text.

A common use case (prior to the introduction of localparam) was to
use macros to define constant values, and to put global constant
values in an include file that gets included by each source file.
This will generate a lot of spurious warnings if we warn about all
redefinitions. Make this new option the default for -Wall.
This commit is contained in:
Martin Whitaker 2017-11-08 19:50:42 +00:00
parent c622d372f9
commit 02daffc97e
5 changed files with 47 additions and 26 deletions

View File

@ -1,4 +1,4 @@
.TH iverilog 1 "Oct 14th, 2017" "" "Version %M.%n%E" .TH iverilog 1 "Nov 8th, 2017" "" "Version %M.%n%E"
.SH NAME .SH NAME
iverilog - Icarus Verilog compiler iverilog - Icarus Verilog compiler
@ -316,8 +316,9 @@ after a \fB\-Wall\fP argument to suppress isolated warning types.
.TP 8 .TP 8
.B all .B all
This enables the anachronisms, implicit, macro-redefinition, portbind, This enables the anachronisms, implicit, macro-replacement, portbind,
select\-range, timescale, and sensitivity\-entire\-array warning categories. select\-range, timescale, and sensitivity\-entire\-array warning
categories.
.TP 8 .TP 8
.B anachronisms .B anachronisms
@ -331,8 +332,11 @@ example, if a scalar wire X is used but not declared in the Verilog
source, this will print a warning at its first use. source, this will print a warning at its first use.
.TP 8 .TP 8
.B macro-redefinition .B macro-redefinition\fI | \fPmacro-replacement
This enables preprocessor warnings when a macro is being redefined. This enables preprocessor warnings when a macro is being redefined.
The first variant prints a warning any time a macro is redefined.
The second variant only prints a warning if the macro text changes.
Use \fBno-macro-redefinition\fP to turn off all warnings of this type.
.TP 8 .TP 8
.B portbind .B portbind

View File

@ -350,7 +350,8 @@ static void build_preprocess_command(int e_flag)
ivlpp_dir, sep, ivlpp_dir, sep,
verbose_flag ? " -v" : "", verbose_flag ? " -v" : "",
e_flag ? "" : " -L", e_flag ? "" : " -L",
strchr(warning_flags, 'r') ? " -Wredef " : "", strchr(warning_flags, 'r') ? " -Wredef-all " :
strchr(warning_flags, 'R') ? " -Wredef-chg " : "",
defines_path, source_path, defines_path, source_path,
compiled_defines_path, compiled_defines_path,
e_flag ? "" : " | "); e_flag ? "" : " | ");
@ -520,14 +521,11 @@ static void process_warning_switch(const char*name)
process_warning_switch("anachronisms"); process_warning_switch("anachronisms");
process_warning_switch("implicit"); process_warning_switch("implicit");
process_warning_switch("implicit-dimensions"); process_warning_switch("implicit-dimensions");
process_warning_switch("macro-replacement");
process_warning_switch("portbind"); process_warning_switch("portbind");
process_warning_switch("select-range"); process_warning_switch("select-range");
process_warning_switch("timescale"); process_warning_switch("timescale");
process_warning_switch("sensitivity-entire-array"); process_warning_switch("sensitivity-entire-array");
process_warning_switch("macro-redefinition");
} else if (strcmp(name,"macro-redefinition") == 0) {
if (! strchr(warning_flags, 'r'))
strcat(warning_flags, "r");
} else if (strcmp(name,"anachronisms") == 0) { } else if (strcmp(name,"anachronisms") == 0) {
if (! strchr(warning_flags, 'n')) if (! strchr(warning_flags, 'n'))
strcat(warning_flags, "n"); strcat(warning_flags, "n");
@ -540,6 +538,12 @@ static void process_warning_switch(const char*name)
} else if (strcmp(name,"implicit-dimensions") == 0) { } else if (strcmp(name,"implicit-dimensions") == 0) {
if (! strchr(warning_flags, 'd')) if (! strchr(warning_flags, 'd'))
strcat(warning_flags, "d"); strcat(warning_flags, "d");
} else if (strcmp(name,"macro-redefinition") == 0) {
if (! strchr(warning_flags, 'r'))
strcat(warning_flags, "r");
} else if (strcmp(name,"macro-replacement") == 0) {
if (! strchr(warning_flags, 'R'))
strcat(warning_flags, "R");
} else if (strcmp(name,"portbind") == 0) { } else if (strcmp(name,"portbind") == 0) {
if (! strchr(warning_flags, 'p')) if (! strchr(warning_flags, 'p'))
strcat(warning_flags, "p"); strcat(warning_flags, "p");
@ -566,12 +570,6 @@ static void process_warning_switch(const char*name)
cp[0] = cp[1]; cp[0] = cp[1];
cp += 1; cp += 1;
} }
} else if (strcmp(name,"no-macro-redefinition") == 0) {
char*cp = strchr(warning_flags, 'r');
if (cp) while (*cp) {
cp[0] = cp[1];
cp += 1;
}
} else if (strcmp(name,"no-floating-nets") == 0) { } else if (strcmp(name,"no-floating-nets") == 0) {
char*cp = strchr(warning_flags, 'f'); char*cp = strchr(warning_flags, 'f');
if (cp) while (*cp) { if (cp) while (*cp) {
@ -590,6 +588,17 @@ static void process_warning_switch(const char*name)
cp[0] = cp[1]; cp[0] = cp[1];
cp += 1; cp += 1;
} }
} else if (strcmp(name,"no-macro-redefinition") == 0) {
char*cp = strchr(warning_flags, 'r');
if (cp) while (*cp) {
cp[0] = cp[1];
cp += 1;
}
cp = strchr(warning_flags, 'R');
if (cp) while (*cp) {
cp[0] = cp[1];
cp += 1;
}
} else if (strcmp(name,"no-portbind") == 0) { } else if (strcmp(name,"no-portbind") == 0) {
char*cp = strchr(warning_flags, 'p'); char*cp = strchr(warning_flags, 'p');
if (cp) while (*cp) { if (cp) while (*cp) {
@ -1322,7 +1331,8 @@ int main(int argc, char **argv)
files. */ files. */
fprintf(iconfig_file, "ivlpp:%s%civlpp %s -L -F\"%s\" -P\"%s\"\n", fprintf(iconfig_file, "ivlpp:%s%civlpp %s -L -F\"%s\" -P\"%s\"\n",
ivlpp_dir, sep, ivlpp_dir, sep,
strchr(warning_flags, 'r') ? "-Wredef" : "", strchr(warning_flags, 'r') ? "-Wredef-all" :
strchr(warning_flags, 'R') ? "-Wredef-chg" : "",
defines_path, compiled_defines_path defines_path, compiled_defines_path
); );

View File

@ -1,7 +1,7 @@
#ifndef IVL_globals_H #ifndef IVL_globals_H
#define IVL_globals_H #define IVL_globals_H
/* /*
* Copyright (c) 1999-2014 Stephen Williams (steve@icarus.com) * Copyright (c) 1999-2017 Stephen Williams (steve@icarus.com)
* *
* This source code is free software; you can redistribute it * This source code is free software; you can redistribute it
* and/or modify it in source code form under the terms of the GNU * and/or modify it in source code form under the terms of the GNU
@ -54,6 +54,7 @@ extern char dep_mode;
extern int verbose_flag; extern int verbose_flag;
extern int warn_redef; extern int warn_redef;
extern int warn_redef_all;
/* This is the entry to the lexer. */ /* This is the entry to the lexer. */
extern int yylex(void); extern int yylex(void);

View File

@ -1,7 +1,7 @@
%option prefix="yy" %option prefix="yy"
%{ %{
/* /*
* Copyright (c) 1999-2016 Stephen Williams (steve@icarus.com) * Copyright (c) 1999-2017 Stephen Williams (steve@icarus.com)
* *
* This source code is free software; you can redistribute it * This source code is free software; you can redistribute it
* and/or modify it in source code form under the terms of the GNU * and/or modify it in source code form under the terms of the GNU
@ -866,7 +866,7 @@ void define_macro(const char* name, const char* value, int keyword, int argc)
*/ */
if (warn_redef) { if (warn_redef) {
prev = def_lookup(name); prev = def_lookup(name);
if (prev) { if (prev && (warn_redef_all || (strcmp(prev->value, value) != 0))) {
emit_pathline(istack); emit_pathline(istack);
fprintf(stderr, "warning: redefinition of macro %s from value '%s' to '%s'\n", fprintf(stderr, "warning: redefinition of macro %s from value '%s' to '%s'\n",
name, prev->value, value); name, prev->value, value);

View File

@ -1,5 +1,5 @@
const char COPYRIGHT[] = const char COPYRIGHT[] =
"Copyright (c) 1999-2015 Stephen Williams (steve@icarus.com)"; "Copyright (c) 1999-2017 Stephen Williams (steve@icarus.com)";
/* /*
* This source code is free software; you can redistribute it * This source code is free software; you can redistribute it
* and/or modify it in source code form under the terms of the GNU * and/or modify it in source code form under the terms of the GNU
@ -100,6 +100,7 @@ FILE *depend_file = NULL;
/* Should we warn about macro redefinitions? */ /* Should we warn about macro redefinitions? */
int warn_redef = 0; int warn_redef = 0;
int warn_redef_all = 0;
static int flist_read_flags(const char*path) static int flist_read_flags(const char*path)
{ {
@ -339,11 +340,14 @@ int main(int argc, char*argv[])
break; break;
} }
case 'W': { case 'W':
if (strcmp(optarg, "redef")==0) if (strcmp(optarg, "redef-all") == 0) {
warn_redef_all = 1;
warn_redef = 1;
} else if (strcmp(optarg, "redef-chg") == 0) {
warn_redef = 1; warn_redef = 1;
break;
} }
break;
case 'v': case 'v':
fprintf(stderr, "Icarus Verilog Preprocessor version " fprintf(stderr, "Icarus Verilog Preprocessor version "
@ -376,7 +380,9 @@ int main(int argc, char*argv[])
" -P<fil> - Read precompiled defines from <fil>\n" " -P<fil> - Read precompiled defines from <fil>\n"
" -v - Verbose\n" " -v - Verbose\n"
" -V - Print version information and quit\n" " -V - Print version information and quit\n"
" -W<cat> - Enable extra ivlpp warning category (e.g. redef)\n", " -W<cat> - Enable extra ivlpp warning category:\n"
" o redef-all - all macro redefinitions\n"
" o redef-chg - macro definition changes\n",
argv[0]); argv[0]);
return flag_errors; return flag_errors;
} }