-
Notifications
You must be signed in to change notification settings - Fork 273
Extend linter to check module dependencies #2196
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,9 +17,7 @@ Author: Daniel Kroening, [email protected] | |
#include <memory> | ||
|
||
#include <ansi-c/ansi_c_language.h> | ||
#include <cpp/cpp_language.h> | ||
#include <java_bytecode/java_bytecode_language.h> | ||
#include <jsil/jsil_language.h> | ||
|
||
#include <goto-programs/goto_convert_functions.h> | ||
#include <goto-programs/goto_inline.h> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
analyses | ||
ansi-c # should go away | ||
cbmc # version.h | ||
java_bytecode | ||
jdiff | ||
goto-analyzer | ||
goto-programs | ||
langapi # should go away | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
java_bytecode/library |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
analyses | ||
ansi-c # should go away | ||
goto-programs | ||
java_bytecode | ||
json | ||
langapi # should go away | ||
library | ||
linking | ||
miniz | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
analyses | ||
ansi-c # should go away | ||
cbmc # version.h and bmc.h | ||
goto-instrument | ||
goto-programs | ||
goto-symex | ||
java_bytecode | ||
jbmc | ||
langapi # should go away | ||
linking | ||
pointer-analysis | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
analyses | ||
cbmc # version.h | ||
java_bytecode | ||
jdiff | ||
goto-diff | ||
goto-instrument | ||
goto-programs | ||
goto-symex # dubious - rewrite_union | ||
langapi # should go away | ||
pointer-analysis | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
miniz | ||
sys # system |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4981,7 +4981,7 @@ def _ClassifyInclude(fileinfo, include, is_system): | |
|
||
|
||
|
||
def CheckIncludeLine(filename, clean_lines, linenum, include_state, error): | ||
def CheckIncludeLine(filename, clean_lines, linenum, include_state, error, module_deps): | ||
"""Check rules that are applicable to #include lines. | ||
|
||
Strings on #include lines are NOT removed from elided line, to make | ||
|
@@ -5029,6 +5029,18 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error): | |
elif not _THIRD_PARTY_HEADERS_PATTERN.match(include): | ||
include_state.include_list[-1].append((include, linenum)) | ||
|
||
# Check module dependencies | ||
module_name = os.path.dirname(filename) | ||
has_src = module_name.find('src/') | ||
if has_src >= 0: | ||
module_name = module_name[has_src+4:] | ||
deps_name = os.path.dirname(include) | ||
if deps_name and module_deps: | ||
may_use = any(deps_name.startswith(module) for module in module_deps) | ||
if not may_use: | ||
error(filename, linenum, 'build/include', 4, | ||
'Module `'+module_name+'` must not use `'+include+'`') | ||
|
||
# We want to ensure that headers appear in the right order: | ||
# 1) for foo.cc, foo.h (preferred location) | ||
# 2) c system files | ||
|
@@ -5141,7 +5153,7 @@ def _GetTextInside(text, start_pattern): | |
|
||
|
||
def CheckLanguage(filename, clean_lines, linenum, file_extension, | ||
include_state, nesting_state, error): | ||
include_state, nesting_state, error, module_deps): | ||
"""Checks rules from the 'C++ language rules' section of cppguide.html. | ||
|
||
Some of these rules are hard to test (function overloading, using | ||
|
@@ -5165,7 +5177,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, | |
|
||
match = _RE_PATTERN_INCLUDE.search(line) | ||
if match: | ||
CheckIncludeLine(filename, clean_lines, linenum, include_state, error) | ||
CheckIncludeLine(filename, clean_lines, linenum, include_state, error, module_deps) | ||
return | ||
|
||
# Reset include state across preprocessor directives. This is meant | ||
|
@@ -6246,7 +6258,7 @@ def CheckForEndl(filename, clean_lines, linenum, error): | |
error(filename, linenum, 'runtime/endl', 4, 'Do not use std::endl') | ||
|
||
def ProcessLine(filename, file_extension, clean_lines, line, | ||
include_state, function_state, nesting_state, error, | ||
include_state, function_state, nesting_state, error, module_deps, | ||
extra_check_functions=[]): | ||
"""Processes a single line in the file. | ||
|
||
|
@@ -6276,7 +6288,7 @@ def ProcessLine(filename, file_extension, clean_lines, line, | |
CheckForMultilineCommentsAndStrings(filename, clean_lines, line, error) | ||
CheckStyle(filename, clean_lines, line, file_extension, nesting_state, error) | ||
CheckLanguage(filename, clean_lines, line, file_extension, include_state, | ||
nesting_state, error) | ||
nesting_state, error, module_deps) | ||
CheckForNonConstReference(filename, clean_lines, line, nesting_state, error) | ||
CheckForNonStandardConstructs(filename, clean_lines, line, | ||
nesting_state, error) | ||
|
@@ -6370,6 +6382,27 @@ def ProcessFileData(filename, file_extension, lines, error, | |
|
||
ResetNolintSuppressions() | ||
|
||
# Load module dependencies | ||
module_deps_file = os.path.join(os.path.dirname(filename), 'module_dependencies.txt') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My mistake - I see sub folders get their own dependencies. |
||
module_deps = [] | ||
if os.path.isfile(module_deps_file): | ||
with open(module_deps_file, 'r') as f: | ||
module_deps = f.read().splitlines() | ||
# strip off comments and whitespace | ||
def strip_off_comments(s): | ||
comment_index = s.find('#') | ||
if comment_index >= 0: | ||
s = s[:comment_index] | ||
s = s.strip() | ||
return s | ||
module_deps = [strip_off_comments(module) for module in module_deps] | ||
# remove empty lines | ||
module_deps = [module for module in module_deps if len(module) != 0] | ||
else: | ||
error(filename, 0, 'build/include', 4, | ||
'module_dependencies.txt not found in `' + | ||
os.path.dirname(filename) + '`') | ||
|
||
CheckForCopyright(filename, lines, error) | ||
CheckForFunctionCommentHeaders(filename, lines, error) | ||
ProcessGlobalSuppresions(lines) | ||
|
@@ -6381,7 +6414,7 @@ def ProcessFileData(filename, file_extension, lines, error, | |
|
||
for line in xrange(clean_lines.NumLines()): | ||
ProcessLine(filename, file_extension, clean_lines, line, | ||
include_state, function_state, nesting_state, error, | ||
include_state, function_state, nesting_state, error, module_deps, | ||
extra_check_functions) | ||
FlagCxx11Features(filename, clean_lines, line, error) | ||
nesting_state.CheckCompletedBlocks(filename, error) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
analyses | ||
ansi-c # should go away | ||
goto-programs | ||
langapi # should go away | ||
pointer-analysis | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
ansi-c | ||
ansi-c/library |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,8 +21,9 @@ Author: Daniel Kroening, [email protected] | |
#include <util/std_types.h> | ||
#include <util/string2int.h> | ||
|
||
#include <ansi-c/gcc_types.h> | ||
|
||
#include "parse_float.h" | ||
#include "../gcc_types.h" | ||
|
||
exprt convert_float_literal(const std::string &src) | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
ansi-c | ||
ansi-c/literals | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
ansi-c | ||
goto-programs | ||
langapi # should go away | ||
linking | ||
literals | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
assembler | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
big-int |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
analyses | ||
ansi-c | ||
cpp | ||
goto-instrument | ||
goto-programs | ||
goto-symex | ||
jsil | ||
langapi # should go away | ||
linking | ||
pointer-analysis | ||
solvers | ||
xmllang | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
analyses | ||
ansi-c | ||
cbmc | ||
clobber | ||
cpp | ||
goto-programs | ||
goto-instrument | ||
java_bytecode # will go away | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the difference between "will go away" and "should go away"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 'will go away' has already a PR filed ;) |
||
langapi # should go away | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
ansi-c | ||
cpp | ||
langapi # should go away | ||
linking | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
ansi-c | ||
analyses | ||
cbmc # version.h | ||
cpp | ||
goto-analyzer | ||
goto-programs | ||
java_bytecode # will go away | ||
langapi # should go away | ||
jsil | ||
json | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
ansi-c | ||
cbmc # version.h | ||
cpp | ||
goto-cc | ||
goto-programs | ||
jsil | ||
json | ||
langapi # should go away | ||
linking | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
analyses | ||
ansi-c | ||
cbmc # version.h | ||
cpp | ||
goto-diff | ||
goto-instrument | ||
goto-programs | ||
goto-symex # dubious - rewrite_union and adjust_float should be moved to goto-programs | ||
java_bytecode # will go away | ||
langapi # should go away | ||
pointer-analysis | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
analyses | ||
ansi-c # should go away | ||
goto-instrument/accelerate | ||
goto-programs | ||
goto-symex | ||
solvers | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
accelerate | ||
analyses | ||
ansi-c | ||
cbmc # version.h | ||
cpp | ||
goto-instrument | ||
goto-programs | ||
langapi # should go away | ||
linking | ||
pointer-analysis | ||
util | ||
wmm |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
goto-instrument | ||
goto-instrument/wmm | ||
goto-programs | ||
linking | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,8 @@ Author: Daniel Kroening, [email protected] | |
|
||
#include <linking/static_lifetime_init.h> | ||
|
||
#include "../rw_set.h" | ||
#include <goto-instrument/rw_set.h> | ||
|
||
#include "fence.h" | ||
|
||
/// returns a unique id (for fresh variables) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
analyses # dubious - concerns call_graph and does_remove_const | ||
ansi-c # should go away | ||
assembler # should go away | ||
goto-programs | ||
goto-symex # dubious - spurious inclusion of symex_target_equation in graphml_witness | ||
json | ||
langapi # should go away | ||
linking | ||
mach-o # system | ||
util | ||
xmllang |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
analyses | ||
goto-programs | ||
goto-symex | ||
langapi # should go away | ||
linking | ||
pointer-analysis | ||
solvers | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
ansi-c # should go away | ||
goto-programs | ||
jsil | ||
langapi # should go away | ||
linking | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
json | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
goto-programs | ||
langapi | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
ansi-c # should go away | ||
goto-programs | ||
langapi # should go away | ||
linking | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
cbmc # version.h | ||
memory-models | ||
langapi # should go away | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
miniz | ||
sys # system |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
nonstd |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
analyses | ||
ansi-c # should go away | ||
goto-programs | ||
langapi # should go away | ||
pointer-analysis | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
solvers/cvc | ||
solvers/flattening # pointer_logic.h | ||
solvers/prop | ||
util |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,8 +26,8 @@ Author: Daniel Kroening, [email protected] | |
|
||
#include "boolbv_type.h" | ||
|
||
#include "../floatbv/float_utils.h" | ||
#include "../lowering/expr_lowering.h" | ||
#include <solvers/floatbv/float_utils.h> | ||
#include <solvers/lowering/expr_lowering.h> | ||
|
||
bool boolbvt::literal( | ||
const exprt &expr, | ||
|
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.
Should this be a warning as well?
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.
I don't think it's worth a warning. It will only affect the module name printed in the error message.