ABataev added a comment. In D71241#1783499 <https://reviews.llvm.org/D71241#1783499>, @hfinkel wrote:
> In D71241#1783444 <https://reviews.llvm.org/D71241#1783444>, @ABataev wrote: > > > In D71241#1782586 <https://reviews.llvm.org/D71241#1782586>, @hfinkel wrote: > > > > > In D71241#1782460 <https://reviews.llvm.org/D71241#1782460>, > > > @JonChesterfield wrote: > > > > > > > > https://clang.llvm.org/docs/InternalsManual.html#the-ast-library > > > > > > > > > > Faithfulness¶ > > > > > The AST intends to provide a representation of the program that is > > > > > faithful to the original source. > > > > > > > > That's pretty convincing. > > > > > > > > > No, you're misinterpreting the intent of the statement. Here's the entire > > > section... > > > > > > > Faithfulness > > > > The AST intends to provide a representation of the program that is > > > > faithful to the original source. We intend for it to be possible to > > > > write refactoring tools using only information stored in, or easily > > > > reconstructible from, the Clang AST. This means that the AST > > > > representation should either not desugar source-level constructs to > > > > simpler forms, or – where made necessary by language semantics or a > > > > clear engineering tradeoff – should desugar minimally and wrap the > > > > result in a construct representing the original source form. > > > > > > > > For example, CXXForRangeStmt directly represents the syntactic form of > > > > a range-based for statement, but also holds a semantic representation > > > > of the range declaration and iterator declarations. It does not contain > > > > a fully-desugared ForStmt, however. > > > > > > > > Some AST nodes (for example, ParenExpr) represent only syntax, and > > > > others (for example, ImplicitCastExpr) represent only semantics, but > > > > most nodes will represent a combination of syntax and associated > > > > semantics. Inheritance is typically used when representing different > > > > (but related) syntaxes for nodes with the same or similar semantics. > > > > > > First, being "faithful" to the original source means both syntax and > > > semantics. I realize that AST is a somewhat-ambiguous term - we have > > > semantic elements in our AST - but Clang's AST is not just some kind of > > > minimized parse tree. The AST even has semantics-only nodes (e.g., for > > > implicit casts). As you can see, the design considerations here are not > > > just "record the local syntactic elements", but require semantic > > > interpretation of these elements. > > > > > > Again, Clang's AST is used for various kinds of static analysis tools, > > > and these depend on having overload resolution correctly performed prior > > > to that analysis. This includes overload resolution that depends on > > > context (whether that's qualifications on `this` or host/device in CUDA > > > or anything else). > > > > > > None of this is to say that we should not record the original spelling of > > > the function call, we should do that *also*, and that should be done when > > > constructing the AST in Sema in addition to performing the variant > > > selection. > > > > > > You are not corret. Check the implementation of decltype, for example > > https://godbolt.org/z/R76Nn. We keep the original decltype in AST, though > > we could easily lower it to the real type. This is the design of AST - keep > > it as much as possible close to the original code and modify it only if it > > is absolutely impossible (again, check > > https://clang.llvm.org/docs/InternalsManual.html#the-ast-library). > > > Yes, but our decltype representation is a semantically-accurate > representation of the source constructs. Your CodeGen approach to variant > selection leads to a semantically-inaccurate representation: it produces a > CallExpr that has a DeclRefExpr to the wrong function declaration. > > In any case, our decltype representation is fairly analogous to what I > proposed above. DecltypeType derives from Type, and stores both the original > expression and the underlying type. If we have an OpenMPVariantCallExpr, it > would also store a deference to the base function definition, but like > desugar() returns the "resolved" regular type, OpenMPVariantCallExpr's > getCallee() would return the resolved function that will be called. But we don't need it. We have all the required information in the AST tree. Each function has associated list of attributes with the context traits and variant function. When you process CallExpr, just check the list of these attributes and return the address of the variant function instead of the original. > > >> Constexprs are not lowered in AST. They are lowered in place where it is >> required. constexpr is just evaluated. It can be evaluated in Sema, if >> required, or in codegen, in the analysis tool. Check >> https://godbolt.org/z/wr9RFk as an example. You will see, the constexprs >> are not evaluated in AST, instead AST tries to do everything to keep them in >> their original form. > > I'm aware of how this works. If we see a need for a lazy evaluation strategy > here we can certainly discuss that. And I agree that we should not drop all > information about the base function. I'm simply saying that's not what > getCallee() should return. However, what you're doing here is not analogous > to the evaluation of constexprs. In short, keeping close to the original > source does justify misrepresenting the semantics. Let's wait the answer from Richard Smith. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71241/new/ https://reviews.llvm.org/D71241 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
