Dead (or untested?) code in the gold plugin

Project Infrastructure LLVM Dev List Archives
1

If I remove this code,

diff --git a/tools/gold/gold-plugin.cpp b/tools/gold/gold-plugin.cpp
index 46a5f46..a8978d5 100644
--- a/tools/gold/gold-plugin.cpp
+++ b/tools/gold/gold-plugin.cpp
@@ -428,8 +428,6 @@ getMinVisibility(GlobalValue::VisibilityTypes A,
     return A;
   if (B == GlobalValue::HiddenVisibility)
     return B;
- if (A == GlobalValue::ProtectedVisibility)
- return A;
   return B;
}

@@ -520,15 +518,9 @@ static ld_plugin_status claim_file_hook(const
ld_plugin_input_file *file,
       Res.IsLinkonceOdr &= GV->hasLinkOnceLinkage();
       Res.Visibility = getMinVisibility(Res.Visibility, GV->getVisibility());
       switch (GV->getVisibility()) {
- case GlobalValue::DefaultVisibility:
- sym.visibility = LDPV_DEFAULT;
- break;
       case GlobalValue::HiddenVisibility:
         sym.visibility = LDPV_HIDDEN;
         break;
- case GlobalValue::ProtectedVisibility:
- sym.visibility = LDPV_PROTECTED;
- break;
       }
     }

all the tests still pass. Is it just dead, or untested?

Thanks,

2

ld_plugin_input_file *file,
       Res.IsLinkonceOdr &= GV->hasLinkOnceLinkage();
       Res.Visibility = getMinVisibility(Res.Visibility, GV->getVisibility());
       switch (GV->getVisibility()) {
- case GlobalValue::DefaultVisibility:
- sym.visibility = LDPV_DEFAULT;
- break;

This part is actually dead since we have initialized sym.visibility to
LDPV_DEFAULT just before the switch. I would still keep it just to
have a fully covered switch. Maybe delete just the assignment?

I have added tests for the other code parts.

Thanks,
Rafael

3

ld_plugin_input_file *file,
       Res.IsLinkonceOdr &= GV->hasLinkOnceLinkage();
       Res.Visibility = getMinVisibility(Res.Visibility, GV->getVisibility());
       switch (GV->getVisibility()) {
- case GlobalValue::DefaultVisibility:
- sym.visibility = LDPV_DEFAULT;
- break;

This part is actually dead since we have initialized sym.visibility to
LDPV_DEFAULT just before the switch. I would still keep it just to
have a fully covered switch. Maybe delete just the assignment?

r267429

I have added tests for the other code parts.

Thank you.