From 02daffc97e34fb86eb18226fc9dbe7c0d598fa03 Mon Sep 17 00:00:00 2001 From: Martin Whitaker Date: Wed, 8 Nov 2017 19:50:42 +0000 Subject: [PATCH] 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. --- driver/iverilog.man.in | 12 ++++++++---- driver/main.c | 34 ++++++++++++++++++++++------------ ivlpp/globals.h | 3 ++- ivlpp/lexor.lex | 4 ++-- ivlpp/main.c | 20 +++++++++++++------- 5 files changed, 47 insertions(+), 26 deletions(-) diff --git a/driver/iverilog.man.in b/driver/iverilog.man.in index 3861690d2..56ba6d201 100644 --- a/driver/iverilog.man.in +++ b/driver/iverilog.man.in @@ -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 iverilog - Icarus Verilog compiler @@ -316,8 +316,9 @@ after a \fB\-Wall\fP argument to suppress isolated warning types. .TP 8 .B all -This enables the anachronisms, implicit, macro-redefinition, portbind, -select\-range, timescale, and sensitivity\-entire\-array warning categories. +This enables the anachronisms, implicit, macro-replacement, portbind, +select\-range, timescale, and sensitivity\-entire\-array warning +categories. .TP 8 .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. .TP 8 -.B macro-redefinition +.B macro-redefinition\fI | \fPmacro-replacement 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 .B portbind diff --git a/driver/main.c b/driver/main.c index d12c40fe1..b723d7ff3 100644 --- a/driver/main.c +++ b/driver/main.c @@ -350,7 +350,8 @@ static void build_preprocess_command(int e_flag) ivlpp_dir, sep, verbose_flag ? " -v" : "", e_flag ? "" : " -L", - strchr(warning_flags, 'r') ? " -Wredef " : "", + strchr(warning_flags, 'r') ? " -Wredef-all " : + strchr(warning_flags, 'R') ? " -Wredef-chg " : "", defines_path, source_path, compiled_defines_path, e_flag ? "" : " | "); @@ -520,14 +521,11 @@ static void process_warning_switch(const char*name) process_warning_switch("anachronisms"); process_warning_switch("implicit"); process_warning_switch("implicit-dimensions"); + process_warning_switch("macro-replacement"); process_warning_switch("portbind"); process_warning_switch("select-range"); process_warning_switch("timescale"); 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) { if (! strchr(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) { if (! strchr(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) { if (! strchr(warning_flags, 'p')) strcat(warning_flags, "p"); @@ -566,12 +570,6 @@ static void process_warning_switch(const char*name) cp[0] = 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) { char*cp = strchr(warning_flags, 'f'); if (cp) while (*cp) { @@ -590,6 +588,17 @@ static void process_warning_switch(const char*name) cp[0] = 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) { char*cp = strchr(warning_flags, 'p'); if (cp) while (*cp) { @@ -1322,7 +1331,8 @@ int main(int argc, char **argv) files. */ fprintf(iconfig_file, "ivlpp:%s%civlpp %s -L -F\"%s\" -P\"%s\"\n", 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 ); diff --git a/ivlpp/globals.h b/ivlpp/globals.h index 1c4cee43d..a1f77e95e 100644 --- a/ivlpp/globals.h +++ b/ivlpp/globals.h @@ -1,7 +1,7 @@ #ifndef 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 * 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 warn_redef; +extern int warn_redef_all; /* This is the entry to the lexer. */ extern int yylex(void); diff --git a/ivlpp/lexor.lex b/ivlpp/lexor.lex index a74a8c357..5ab3e55de 100644 --- a/ivlpp/lexor.lex +++ b/ivlpp/lexor.lex @@ -1,7 +1,7 @@ %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 * 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) { prev = def_lookup(name); - if (prev) { + if (prev && (warn_redef_all || (strcmp(prev->value, value) != 0))) { emit_pathline(istack); fprintf(stderr, "warning: redefinition of macro %s from value '%s' to '%s'\n", name, prev->value, value); diff --git a/ivlpp/main.c b/ivlpp/main.c index b4909b647..19ef09bde 100644 --- a/ivlpp/main.c +++ b/ivlpp/main.c @@ -1,5 +1,5 @@ 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 * 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? */ int warn_redef = 0; +int warn_redef_all = 0; static int flist_read_flags(const char*path) { @@ -339,11 +340,14 @@ int main(int argc, char*argv[]) break; } - case 'W': { - if (strcmp(optarg, "redef")==0) - warn_redef = 1; - break; - } + case 'W': + if (strcmp(optarg, "redef-all") == 0) { + warn_redef_all = 1; + warn_redef = 1; + } else if (strcmp(optarg, "redef-chg") == 0) { + warn_redef = 1; + } + break; case 'v': fprintf(stderr, "Icarus Verilog Preprocessor version " @@ -376,7 +380,9 @@ int main(int argc, char*argv[]) " -P - Read precompiled defines from \n" " -v - Verbose\n" " -V - Print version information and quit\n" - " -W - Enable extra ivlpp warning category (e.g. redef)\n", + " -W - Enable extra ivlpp warning category:\n" + " o redef-all - all macro redefinitions\n" + " o redef-chg - macro definition changes\n", argv[0]); return flag_errors; }