Skip to content

Commit

Permalink
Fix Remove_Redundant_Decls removing necessary decls
Browse files Browse the repository at this point in the history
PrettyPrint::Contains_From_LineCol had a bug in which Decls that do not
contain themselves were marked as such because two files ended up having
the same FileID because of preprocessed notation (example: # 1 "file").
Fix this by dropping this function in favor of
Range_Fully_Contains_Range, which uses the expanded location.

Signed-off-by: Giuliano Belinassi <gbelinassi@suse.de>
  • Loading branch information
giulianobelinassi committed Sep 25, 2024
1 parent 4a9b5e3 commit 5073600
Show file tree
Hide file tree
Showing 10 changed files with 46,605 additions and 49 deletions.
8 changes: 4 additions & 4 deletions libcextract/FunctionDepsFinder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ void FunctionDependencyFinder::Remove_Redundant_Decls(void)
PrettyPrint::Get_Source_Text(type_range) != "") {

/* Using .fullyContains() fails in some declarations. */
if (PrettyPrint::Contains_From_LineCol(range, type_range)) {
if (Range_Fully_Contains_Range(AST, range, type_range)) {
closure.Remove_Decl(typedecl);
}
}
Expand Down Expand Up @@ -195,7 +195,7 @@ void FunctionDependencyFinder::Remove_Redundant_Decls(void)
SourceRange type_range = typedecl->getSourceRange();

/* Using .fullyContains() fails in some declarations. */
if (PrettyPrint::Contains_From_LineCol(range, type_range)) {
if (Range_Fully_Contains_Range(AST, range, type_range)) {
closure.Remove_Decl(typedecl);
}
}
Expand Down Expand Up @@ -266,8 +266,8 @@ void FunctionDependencyFinder::Remove_Redundant_Decls(void)
*/
if (PrettyPrint::Get_Source_Text(decl->getSourceRange()) != "" &&
PrettyPrint::Get_Source_Text(prev->getSourceRange()) != "" &&
PrettyPrint::Contains_From_LineCol(decl->getSourceRange(),
prev->getSourceRange())) {
Range_Fully_Contains_Range(AST, decl->getSourceRange(),
prev->getSourceRange())) {
/*
* If the prev and the current decl have the same start LoC, but
* different ending, remove the prev from the closure and set the
Expand Down
11 changes: 11 additions & 0 deletions libcextract/LLVMMisc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -315,3 +315,14 @@ bool Is_Decl_Equivalent_To(Decl *a, Decl *b)

return a_str == b_str;
}

bool Range_Fully_Contains_Range(ASTUnit *ast, const SourceRange &a,
const SourceRange &b)
{
SourceManager &SM = ast->getSourceManager();

SourceRange a_exp(SM.getExpansionLoc(a.getBegin()), SM.getExpansionLoc(a.getEnd()));
SourceRange b_exp(SM.getExpansionLoc(b.getBegin()), SM.getExpansionLoc(b.getEnd()));

return a_exp.fullyContains(b_exp);
}
3 changes: 3 additions & 0 deletions libcextract/LLVMMisc.hh
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,6 @@ DeclContextLookupResult Get_Decl_From_Symtab(ASTUnit *ast, const StringRef &name

/** Check if two Decls are equivalent. */
bool Is_Decl_Equivalent_To(Decl *a, Decl *b);

bool Range_Fully_Contains_Range(ASTUnit *ast, const SourceRange &a,
const SourceRange &b);
41 changes: 0 additions & 41 deletions libcextract/PrettyPrint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -309,47 +309,6 @@ void PrettyPrint::Debug_SourceLoc(const SourceLocation &loc)
loc.dump(AST->getSourceManager());
}

bool PrettyPrint::Contains_From_LineCol(const SourceRange &a, const SourceRange &b)
{
SourceManager &SM = AST->getSourceManager();
PresumedLoc a_begin = SM.getPresumedLoc(a.getBegin());
PresumedLoc a_end = SM.getPresumedLoc(a.getEnd());
PresumedLoc b_begin = SM.getPresumedLoc(b.getBegin());
PresumedLoc b_end = SM.getPresumedLoc(b.getEnd());

assert(a_begin.getFileID() == a_end.getFileID());
assert(b_begin.getFileID() == b_end.getFileID());

if (a_begin.getFileID() != b_begin.getFileID()) {
/* Files are distinct, thus we can't easily determine which comes first. */
return false;
}

bool a_begin_smaller = false;
bool b_end_smaller = false;

if ((a_begin.getLine() < b_begin.getLine()) ||
(a_begin.getLine() == b_begin.getLine() && a_begin.getColumn() <= b_begin.getColumn())) {
a_begin_smaller = true;
}

if ((b_end.getLine() < a_end.getLine()) ||
(b_end.getLine() == a_end.getLine() && b_end.getColumn() <= a_end.getColumn())) {
b_end_smaller = true;
}

return a_begin_smaller && b_end_smaller;
}

bool PrettyPrint::Contains(const SourceRange &a, const SourceRange &b)
{
if (a.fullyContains(b)) {
return true;
}

return Contains_From_LineCol(a, b);
}

/** Compare if SourceLocation a is after SourceLocation b in the source code. */
bool PrettyPrint::Is_After(const SourceLocation &a, const SourceLocation &b)
{
Expand Down
4 changes: 0 additions & 4 deletions libcextract/PrettyPrint.hh
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,6 @@ class PrettyPrint

static void Debug_SourceLoc(const SourceLocation &loc);

static bool Contains_From_LineCol(const SourceRange &a, const SourceRange &b);

static bool Contains(const SourceRange &a, const SourceRange &b);

static inline void Set_AST(ASTUnit *ast)
{
AST = ast;
Expand Down
Loading

0 comments on commit 5073600

Please sign in to comment.