Fix Remove_Redundant_Decls removing necessary decls

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 <[email protected]>
SUSE · Sep 25, 2024 · 6b9a4ea · 6b9a4ea
1 parent 4a9b5e3
commit 6b9a4ea
Show file tree

Hide file tree

Showing 10 changed files with 46,605 additions and 49 deletions.
diff --git a/libcextract/FunctionDepsFinder.cpp b/libcextract/FunctionDepsFinder.cpp
@@ -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);
             }
           }
@@ -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);
         }
       }
@@ -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

diff --git a/libcextract/LLVMMisc.cpp b/libcextract/LLVMMisc.cpp
@@ -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);
+}
diff --git a/libcextract/LLVMMisc.hh b/libcextract/LLVMMisc.hh
@@ -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);
diff --git a/libcextract/PrettyPrint.cpp b/libcextract/PrettyPrint.cpp
@@ -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)
 {

diff --git a/libcextract/PrettyPrint.hh b/libcextract/PrettyPrint.hh
@@ -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;