martong added a comment.

In D59665#1438911 <https://reviews.llvm.org/D59665#1438911>, @a_sidorin wrote:

> Hi Shafik,
>
> Honestly, I was always wondering what does HandleNameConflict actually do. 
> Its implementation in ASTImporter is trivial and I don't see any of its 
> overrides in LLDB code too. Why do we check its result to be non-empty is a 
> question to me as well. And the more I look at it (and the bug you pointed in 
> it), the more I think there is something wrong with it. Maybe it is better to 
> just remove it at all? I hope LLDB developers have more knowledge about it. 
> Shafik, Davide @davide , do you know its actual purpose?


As to my best knowlege, HandleNameConflict is not overridden in LLDB (@davide , 
@shafik please correct my if I am wrong).
However, I would not remove it, I'd rather fix the problems we have around it.
I think via HandleNameConflict we could implement different strategies about 
how to handle ODR errors and perhaps that was the original intention to use it 
for. With CTU (and probably with LLDB too) this could be really helpful: 
Recently I have discovered that during the CTU analysis of Xerces there are 
real ODR errors on a typedef, but it would be nice if we could just keep on 
going with the inlining of a certain function. So, I see three possible 
different strategies to handle name conflicts:

1. Conservative. In case of name conflict propagate the error. (The function 
will not be inlined because of a typedef ODR error.)
2. Liberal. In case of name conflict create a new node with the same name and 
do not propagate the error. (This may give erroneous results if a lookup is 
performed, but most SA checkers does not do lookup)
3. Rename. In case of name conflict create a new node with a different name 
(e.g. with a prefix of the TU's name). Do not propagate the error. (This would 
result the most sane AST, but this is perhaps the most difficult to implement, 
plus a mapping between old and new names has to be given.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59665/new/

https://reviews.llvm.org/D59665



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to