]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
Remove best_symbol and better_symbol
authorTom Tromey <tom@tromey.com>
Tue, 13 May 2025 23:53:27 +0000 (17:53 -0600)
committerTom Tromey <tom@tromey.com>
Sat, 7 Feb 2026 14:38:46 +0000 (07:38 -0700)
This patch removes best_symbol and better_symbol.

Before some c-exp.y changes this year, best_symbol was the source of
excess CU expansion, because searches that include SEARCH_VAR_DOMAIN
will keep going until a symbol is found in that domain, or until all
matches have been exhausted.  So, for instance a "SEARCH_VFT" search
for "int" will, most likely, expand all CUs -- even though the
argument implies that a type would be suitable.

Nowadays, best_symbol is more of an aesthetic offense and abstraction
violation, encoding a workaround for a C++ expression lookup bug into
a low-level lookup function.

This patch changes the the C parser to simply do two lookups,
prioritizing SEARCH_VAR_DOMAIN.  This way, it gets the result it is
presumably looking for, without causing excessive expansion (if that's
currently possible) and without pushing its requirements into generic
code.

Note that the special case for "stub" symbols remains.  This one is
less problematic, at least for the time being, as such symbols aren't
common.  (Kevin does have a patch pending in this area but I don't
believe this change should affect it, as the stub symbol case remains
unchanged.)

New in v2: addressed review comments and reworded the commit text.
v1 is here:

https://inbox.sourceware.org/gdb-patches/87345e71b1.fsf@tromey.com/T/#meb7622f6d4d5bb0c1cd6170d875b37a0ceb9a1ed

Regression tested on x86-64 Fedora 40.

gdb/block.c
gdb/block.h
gdb/c-exp.y
gdb/symtab.c
gdb/symtab.h

index 730e4580a5b714f322c28f909b4c9cfad735369e..2860e2cb1461f284f108e3055ffd134662219957 100644 (file)
@@ -583,46 +583,6 @@ block_iterator_next (struct block_iterator *iterator)
   return block_iter_match_step (iterator, false);
 }
 
-/* See block.h.  */
-
-bool
-best_symbol (struct symbol *a, const domain_search_flags domain)
-{
-  if (a->loc_class () == LOC_UNRESOLVED)
-    return false;
-
-  if ((domain & SEARCH_VAR_DOMAIN) != 0)
-    return a->domain () == VAR_DOMAIN;
-
-  return a->matches (domain);
-}
-
-/* See block.h.  */
-
-struct symbol *
-better_symbol (struct symbol *a, struct symbol *b,
-              const domain_search_flags domain)
-{
-  if (a == NULL)
-    return b;
-  if (b == NULL)
-    return a;
-
-  if (a->matches (domain) && !b->matches (domain))
-    return a;
-
-  if (b->matches (domain) && !a->matches (domain))
-    return b;
-
-  if (a->loc_class () != LOC_UNRESOLVED && b->loc_class () == LOC_UNRESOLVED)
-    return a;
-
-  if (b->loc_class () != LOC_UNRESOLVED && a->loc_class () == LOC_UNRESOLVED)
-    return b;
-
-  return a;
-}
-
 /* See block.h.
 
    Note that if NAME is the demangled form of a C++ symbol, we will fail
@@ -682,6 +642,9 @@ best_symbol_tracker::search (compunit_symtab *symtab,
 {
   for (symbol *sym : block_iterator_range (block, &name))
     {
+      if (!sym->matches (domain))
+       continue;
+
       /* With the fix for PR gcc/debug/91507, we get for:
         ...
         extern char *zzz[];
@@ -703,31 +666,22 @@ best_symbol_tracker::search (compunit_symtab *symtab,
         doesn't work either because the type of the decl does not specify a
         size.
 
-        To fix this, we prefer def over decl in best_symbol and
-        better_symbol.
+        To fix this, we prefer definitions over declarations.
 
         In absence of the gcc fix, both def and decl have type char *[], so
         the only option to make this work is improve the fallback to use the
         size of the minimal symbol.  Filed as PR exp/24989.  */
-      if (best_symbol (sym, domain))
+      if (sym->is_definition ())
        {
          best_symtab = symtab;
          currently_best = { sym, block };
          return true;
        }
 
-      /* This is a bit of a hack, but 'matches' might ignore
-        STRUCT vs VAR domain symbols.  So if a matching symbol is found,
-        make sure there is no "better" matching symbol, i.e., one with
-        exactly the same domain.  PR 16253.  */
-      if (sym->matches (domain))
+      if (currently_best.symbol == nullptr)
        {
-         symbol *better = better_symbol (sym, currently_best.symbol, domain);
-         if (better != currently_best.symbol)
-           {
-             best_symtab = symtab;
-             currently_best = { better, block };
-           }
+         best_symtab = symtab;
+         currently_best = { sym, block };
        }
     }
 
index b84ca12c35a6f834949e61694fee1ca65fc7c606..a859d610d0e1c328108f93ca91989a961e16929a 100644 (file)
@@ -659,16 +659,6 @@ block_iterator_range (const block *block,
   return iterator_range<block_iterator_wrapper> (std::move (begin));
 }
 
-/* Return true if symbol A is the best match possible for DOMAIN.  */
-
-extern bool best_symbol (struct symbol *a, const domain_search_flags domain);
-
-/* Return symbol B if it is a better match than symbol A for DOMAIN.
-   Otherwise return A.  */
-
-extern struct symbol *better_symbol (struct symbol *a, struct symbol *b,
-                                    const domain_search_flags domain);
-
 /* Search BLOCK for symbol NAME in DOMAIN.  */
 
 extern struct symbol *block_lookup_symbol (const struct block *block,
@@ -676,8 +666,8 @@ extern struct symbol *block_lookup_symbol (const struct block *block,
                                           const domain_search_flags domain);
 
 /* When searching for a symbol, the "best" symbol is preferred over
-   one that is merely acceptable.  See 'best_symbol'.  This class
-   keeps track of this distinction while searching.  */
+   one that is merely acceptable.  This class keeps track of this
+   distinction while searching.  */
 
 struct best_symbol_tracker
 {
index 9a37eb5fb692e91d3466623984623f52769688a7..33e1a3847c89a9986404998657bc96939ce4a931 100644 (file)
@@ -3130,8 +3130,14 @@ classify_name (struct parser_state *par_state, const struct block *block,
 
   std::string copy = copy_name (yylval.sval);
 
-  bsym = lookup_symbol (copy.c_str (), block, SEARCH_VFT,
+  /* Prefer variables over functions or types.  This preserves some
+     historical parser behavior.  */
+  bsym = lookup_symbol (copy.c_str (), block, SEARCH_VAR_DOMAIN,
                        &is_a_field_of_this);
+  if (bsym.symbol == nullptr)
+    bsym = lookup_symbol (copy.c_str (), block,
+                         SEARCH_FUNCTION_DOMAIN | SEARCH_TYPE_DOMAIN,
+                         &is_a_field_of_this);
 
   if (bsym.symbol && bsym.symbol->loc_class () == LOC_BLOCK)
     {
index 35e380980b567722c08854c2c0880839e79d087f..7f9f0fb24a98b3ffcacb861c708579bf06dd4407 100644 (file)
@@ -2589,7 +2589,7 @@ lookup_global_symbol (const char *name,
       sym = lookup_symbol_in_block (name,
                                    symbol_name_match_type::FULL,
                                    global_block, domain);
-      if (sym != NULL && best_symbol (sym, domain))
+      if (sym != nullptr && sym->is_definition ())
        return { sym, global_block };
     }
 
@@ -2603,10 +2603,16 @@ lookup_global_symbol (const char *name,
 
   block_symbol bs
     = lookup_global_or_static_symbol (name, GLOBAL_BLOCK, objfile, domain);
-  if (better_symbol (sym, bs.symbol, domain) == sym)
-    return { sym, global_block };
-  else
+  /* Prefer a definition over a declaration.  (At this point, SYM is
+     either nullptr or a declaration.)  */
+  if (bs.symbol != nullptr && bs.symbol->is_definition ())
+    return bs;
+  /* Prefer a declaration if that's all we have.  */
+  if (bs.symbol != nullptr && sym == nullptr)
     return bs;
+  /* Return either an empty result or the declaration we found in the
+     "local" global block.  */
+  return { sym, global_block };
 }
 
 /* See symtab.h.  */
index 8f0cc728410d1315166c98f458cc7be7b4fc3ff9..63741fb7fa7f8ed483ebde314e4977ae10dba676 100644 (file)
@@ -1288,6 +1288,13 @@ struct symbol : public general_symbol_info, public allocate_on_obstack<symbol>
     return this->impl ().loc_class;
   }
 
+  /* Return true if the symbol is not LOC_UNRESOLVED -- that is, if it
+     is a definition and not just a declaration.  */
+  bool is_definition () const
+  {
+    return loc_class () != LOC_UNRESOLVED;
+  }
+
   /* Return true if this symbol's domain matches FLAGS.  */
   bool matches (domain_search_flags flags) const;