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

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241