[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

Johannes Doerfert via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 12 15:33:32 PST 2019


jdoerfert added a comment.

In D71241#1782614 <https://reviews.llvm.org/D71241#1782614>, @ABataev wrote:

> Actually, early resolution will break tbe tools, not help them. It will definitely break clangd, for example. The user will try to navigate to originally called function `base` and instead he will be redirected to the function `hst`.


And that is a good thing. Even if you argue it is not, *only* in the Sema solution the tools have *all* the information available to redirect to the base or variant function.

In D71241#1782612 <https://reviews.llvm.org/D71241#1782612>, @ABataev wrote:

> In D71241#1782504 <https://reviews.llvm.org/D71241#1782504>, @jdoerfert wrote:
>
> > Here, both the original callee (`wrong_ast`) and the actual callee `cpu` are shown at the call site.
> >
> > Why would we not want that?
>
>
> You have wron idea about AST representation. If something is not printed in dump, it does not mean it does nit exist in AST.


This is plain insulting (again) and beyond the point. I just shown you that the proposed solution has *all* the information in the AST available to be used by tools, codegen, ... I did so because you claimed that would not be the case, e.g. the AST would not represent the program faithfully. As you see, all the original information is available. However, you still refuse to acknowledge that and instead try to discredit me. I am tired of this kind of "discussion", we went down this road before and, as it was back then, there is nothing to be gained. It is harmful for the community and it is insulting towards me.

---

While we talk a lot about what you think is bad about this solution it seems we ignore the problems in the current one. Let me summarize a few:

- Take https://godbolt.org/z/XCjQUA where the wrong function is called in the target region (because the "hack" to inject code in the wrong definition is not applicable).
- Take https://godbolt.org/z/Yi9Lht where the wrong function is called on the host (no there is *no* alias hidden)
- Take https://godbolt.org/z/2evvtN which shows that the alias solution is incompatible with linking.
- Take the `construct` context selector and the `begin/end declare variant` construct which both cannot be implemented with aliases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241





More information about the cfe-commits mailing list