With this extension the effects of omp begin declare variant will be
applied to template function declarations. The behavior is opt-in and
controlled by the extension(allow_templates) trait. While generally
useful, this will enable us to implement complex math function calls by
overloading the templates of the standard library with the ones in
libc++.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Nice. What makes it an extension? 5.0 / 2.3.5 claims "and where variant-func-id is the name of a function variant that is either a base language identifier or, for C++, a template-id." which suggests this could be always-on
clang/lib/Sema/SemaOpenMP.cpp | ||
---|---|---|
5916 | tabs! |
This is for begin/end declare variant, a 5.1 feature. There it says:
The begin declare variant directive associates the context selector in the match clause with each function definition in declaration-definition-seq.
So it applies to function definitions between the begin and end. With the extension it applies to template function definitions too.
This looks good to me. I can't claim to have read the test case in detail. Some style suggestions inline around the vector, but they're not blocking.
clang/include/clang/Sema/Sema.h | ||
---|---|---|
10042 | SmallVectorImpl => SmallVector? | |
clang/lib/Sema/SemaOpenMP.cpp | ||
5871–5876 | Return SmallVector<FunctionDecl> by value instead of via the reference? Makes it clear that we aren't adding to an existing vector | |
5916 | This turned out to be how phabricator represents changes in indent. Unfortunate that the symbol is the one I usually see used for tabs. | |
clang/test/AST/ast-dump-openmp-begin-declare-variant_template_2.cpp | ||
58 | I'd like a different way for us to test this stuff. Realistically impossible to spot errors in 200 lines of tree. Don't have a suggestion at this time. |
clang/include/clang/Sema/Sema.h | ||
---|---|---|
10042 | It is a good practice to pass SmallVectors as SmallVectorImpl to make it independent of concrete SmallVector template specialization. |
clang/include/clang/Sema/Sema.h | ||
---|---|---|
10042 | We usually use the Impl versions for the APIs to avoid hardcoding the "size" everywhere. | |
clang/lib/Sema/SemaOpenMP.cpp | ||
5871–5876 | Hm, I guess we can do that, it will cause some more copies. FWIW, we are adding to an existing vector if bases is not empty. I mean, if you provide an empty vector we add and if there is something in the vector we add. | |
clang/test/AST/ast-dump-openmp-begin-declare-variant_template_2.cpp | ||
58 | I manually checked it, and now we can verify changes. At the end of the day checking this by hand is actually not too bad. The trick is that you can skip most of it. You go to the test function return value at the very end. Each call expression there should call a function that returns 0. So follow the callee and check the return there (nothing else anyway). I used this scheme for all tests and once you did verify one it is really easy to do the others. Assuming my input is proper ;) |
SmallVectorImpl => SmallVector?