martong added a comment.

In D68634#1701192 <https://reviews.llvm.org/D68634#1701192>, @martong wrote:

> In D68634#1700591 <https://reviews.llvm.org/D68634#1700591>, @a_sidorin wrote:
>
> > Hello Balasz,
> >  In my opinion, importing and setting the attributes should be handled by 
> > the stuff used in InitializeImportedDecl(). Can we extend it or reuse the 
> > code? It will allow us not to miss the required actions while importing a 
> > new function too.
>
>
> I was thinking about that too, but it turned out to be a way more intrusive 
> change.
>
> We have to work with the most recent existing decl (`PrevDecl`) of the 
> FunctionDecl whose attribute we currently import.
>  We can get the `PrevDecl` only with the help of the lookup. We can't get it 
> from the `ToD` param of `InitializeImportedDecl` because by the time when we 
> call `InitializeImportedDecl` the redecl chain is not connected. So, we 
> should pass `PrevDecl` then into `GetImportedOrCreateDecl`.  It would require 
> to change all call expressions of `GetImportedOrCreateDecl` (58 occurences). 
> Besides, we would use the `PrevDecl` only in case of inheritable attributes, 
> only they require this special care. Note that there are a bunch of 
> non-inheritable attributes which are already handled correctly in 
> `InitializeImportedDecl`.


Actually, giving it some more thought, I realized it would make sense to pass 
the `PrevDecl` into `GetImportedOrCreateDecl` because that way we could connect 
the redecl chain right after the new node is created, and before importing any 
other node. That could facilitate structural eq check in the import of 
subsequent dependent nodes (see https://github.com/Ericsson/clang/issues/506).
Anyway, this change would be quite a big change compared to the original small 
patch we have here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68634



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

Reply via email to