<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jun 8, 2015 at 7:31 AM, Andrey Bokhanko <span dir="ltr"><<a href="mailto:andreybokhanko@gmail.com" target="_blank">andreybokhanko@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">In <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10187-23184399&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=foQ3c1FUKjanWp4BwzZmqEOTtarA7qIFUkJkW5qtS9E&s=d9i8r1UOWCibNr6V5ZrktjW0WO2-5Ng0SLWpMp1mq7w&e=" target="_blank">http://reviews.llvm.org/D10187#184399</a>, @rsmith wrote:<br>
<br>
> I think the right way to handle this would be to filter out declarations that are neither `FunctionDecl`s nor `VarDecl`s after performing the lookup. (According to the specification, we should also filter out declarations with internal linkage.)<br>
<br>
<br>
</span>OK -- I did as you suggested. Please review updated patch.</blockquote><div><br></div><div>That looks fine (although perhaps keep the creation of the attribute outside the `if` so you don't need to duplicate it in both arms).</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">
> We should also filter out internal-linkage declarations and non-TU-scope declarations when we perform the deferred handling of `ExtnameUndeclaredIdentifiers` in `ActOnVariableDeclarator` and `ActOnFunctionDeclarator`.<br>
<br>
<br>
</span>This probably belongs to a separate patch (for starters, I don't have a test demonstrating that compiler is currently acting incorrectly).</blockquote><div><br></div><div>That sounds fine. Here's a trivial testcase (it's easy to cook up more, the bug is quite blatant):</div><div><br></div><div><div>#pragma redefine_extname foo bar</div><div>int f() {</div><div>  int foo = 0;</div><div>  return foo;</div><div>}</div><div>int foo() { return 1; }</div></div><div><br></div><div>The redefine_extname gets applied to the local variable 'foo', and the global function 'foo' does not get renamed.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">
> That said, I'm not sure why your testcase was failing -- `LookupSingleName` in `LookupOrdinaryName` mode in C should not find tag names, so we should have deferred adding the attribute already. Is a `typedef` missing from the testcase, perhaps?<br>
<br>
<br>
</span>You mean modifying it like this:<br>
<br>
typedef struct {<br>
<br>
  int f;<br>
<br>
} statvfs64;<br>
<br>
?<br>
<br>
Then compiler issue an error: "redefine_extname.c:10:5: error: redefinition of 'statvfs64' as different kind of symbol". No errors are issued in the original test.</blockquote><div><br></div><div>Your current test passes for me without your patch, so you need to change it *somehow*.</div></div></div></div>