[PATCH] D124702: [MSVC] Add support for pragma function

This revision was automatically updated to reflect the committed changes. Closed by commit rG3946de0456fc: [MSVC] Add support for pragma function (authored by steplong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124702/new/

[PATCH] D124702: [MSVC] Add support for pragma function

steplong updated this revision to Diff 428981. steplong added a comment. - Updated line in docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124702/new/ https://reviews.llvm.org/D124702 Files: clang/docs/ReleaseNotes.rst

[PATCH] D124702: [MSVC] Add support for pragma function

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, thanks for all the work here! Comment at: clang/docs/ReleaseNotes.rst:272 +- Added support for MSVC's pragma function, which tells the compiler to + generate calls to functions

[PATCH] D124702: [MSVC] Add support for pragma function

steplong updated this revision to Diff 428941. steplong added a comment. - Added test case for the `getRedeclContext()` case - Brought back `getRedeclContext()` - Clang-formatted - Added description in docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D124702: [MSVC] Add support for pragma function

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaAttr.cpp:1070 +SourceLocation Loc, const llvm::SmallVectorImpl ) { + if (!CurContext->isFileContext()) { +Diag(Loc, diag::err_pragma_expected_file_scope) << "function"; It looks like we

[PATCH] D124702: [MSVC] Add support for pragma function

aaron.ballman added inline comments. Comment at: clang/test/Preprocessor/pragma_microsoft.c:210 +#pragma function(main) // expected-warning {{'main' is not a recognized builtin; consider including }} +#pragma function(//

[PATCH] D124702: [MSVC] Add support for pragma function

steplong added inline comments. Comment at: clang/test/Preprocessor/pragma_microsoft.c:210 +#pragma function(main) // expected-warning {{'main' is not a recognized builtin; consider including }} +#pragma function(// expected-warning

[PATCH] D124702: [MSVC] Add support for pragma function

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Thanks for all the hard work on this, LGTM! Can you please add a release note to clang/docs/ReleaseNotes.rst for the new functionality (probably best under the `New Pragmas in

[PATCH] D124702: [MSVC] Add support for pragma function

steplong updated this revision to Diff 428657. steplong added a comment. - Renamed `AddRangeBasedNoBuiltin` to `AddImplicitMSFunctionNoBuiltinAttr` - Renamed `err_pragma_intrinsic_function_scope` to `err_pragma_expected_file_scope` - Added comment about MSVC accepting struct test - Removed

[PATCH] D124702: [MSVC] Add support for pragma function

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaAttr.cpp:1070 +SourceLocation Loc, const llvm::SmallVectorImpl ) { + if (!CurContext->getRedeclContext()->isFileContext()) { +Diag(Loc, diag::err_pragma_intrinsic_function_scope) << "function";

[PATCH] D124702: [MSVC] Add support for pragma function

steplong updated this revision to Diff 428456. steplong added a comment. - Changed pragma-ms-functions.c to only check that the no-builtin attributes are added to the functions - Moved pragma handler to Parser - Changed scope check to only file scope. MSVC accepts stuff like that we don't:

[PATCH] D124702: [MSVC] Add support for pragma function

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaAttr.cpp:1070 +SourceLocation Loc, const llvm::SmallSetVector ) { + if (!CurContext->getRedeclContext()->isFileContext()) { +Diag(Loc, diag::err_pragma_intrinsic_function_scope);

[PATCH] D124702: [MSVC] Add support for pragma function

steplong added inline comments. Comment at: clang/lib/Sema/SemaAttr.cpp:1070 +SourceLocation Loc, const llvm::SmallSetVector ) { + if (!CurContext->getRedeclContext()->isFileContext()) { +Diag(Loc, diag::err_pragma_intrinsic_function_scope);

[PATCH] D124702: [MSVC] Add support for pragma function

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaAttr.cpp:1070 +SourceLocation Loc, const llvm::SmallSetVector ) { + if (!CurContext->getRedeclContext()->isFileContext()) { +Diag(Loc, diag::err_pragma_intrinsic_function_scope);

[PATCH] D124702: [MSVC] Add support for pragma function

steplong added inline comments. Comment at: clang/lib/Sema/SemaAttr.cpp:1070 +SourceLocation Loc, const llvm::SmallSetVector ) { + if (!CurContext->getRedeclContext()->isFileContext()) { +Diag(Loc, diag::err_pragma_intrinsic_function_scope);

[PATCH] D124702: [MSVC] Add support for pragma function

steplong updated this revision to Diff 428375. steplong edited the summary of this revision. steplong added a comment. - Clang-formatted patch - Split out the pragma intrinsic stuff - Replaced SmallSetVector in pragma handler with SmallVector. MSNoBuiltins is still a SmallSetVector in Sema. -

[PATCH] D124702: [MSVC] Add support for pragma function

aaron.ballman added inline comments. Comment at: clang/lib/Parse/ParsePragma.cpp:3578 + + if (Tok.isNot(tok::l_paren)) { +PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_lparen) steplong wrote: > aaron.ballman wrote: > > Can we use

[PATCH] D124702: [MSVC] Add support for pragma function

steplong added inline comments. Comment at: clang/lib/Parse/ParsePragma.cpp:3578 + + if (Tok.isNot(tok::l_paren)) { +PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_lparen) aaron.ballman wrote: > Can we use `ExpectAndConsume()` here instead? I don't

[PATCH] D124702: [MSVC] Add support for pragma function

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:988 +def err_pragma_intrinsic_function_scope : Error< + "'#pragma function/intrinsic' can only appear outside a function, at the global level">; +

[PATCH] D124702: [MSVC] Add support for pragma function

steplong updated this revision to Diff 427716. steplong added a reviewer: aaron.ballman. steplong added a comment. Changed it to use SmallSetVector instead of SmallVector. Had to use an temporary SmallVector in AddRangeBasedNoBuiltin() because I'm not sure how to get a pointer to StringRef.

[PATCH] D124702: [MSVC] Add support for pragma function

steplong added inline comments. Comment at: clang/lib/Sema/SemaAttr.cpp:1079 + MSFunctionNoBuiltins.insert(MSFunctionNoBuiltins.end(), + NoBuiltins.begin(), NoBuiltins.end()); +} hans wrote: > steplong wrote: > > hans wrote: > > >

[PATCH] D124702: [MSVC] Add support for pragma function

steplong added a comment. In D124702#3496534 , @hans wrote: > It needs tests for the warnings about badly formed pragmas, and for pragmas > outside file/namespace scope. Appreciate the comments. I have a couple of questions: Is the tests for badly

[PATCH] D124702: [MSVC] Add support for pragma function

hans added a comment. It needs tests for the warnings about badly formed pragmas, and for pragmas outside file/namespace scope. Comment at: clang/lib/Parse/ParsePragma.cpp:3561 << "intrinsic"; return; } steplong wrote: > hans wrote: > > since

[PATCH] D124702: [MSVC] Add support for pragma function

steplong updated this revision to Diff 427127. steplong added a comment. Error if pragma function/intrinsic is used inside a function Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124702/new/ https://reviews.llvm.org/D124702 Files:

[PATCH] D124702: [MSVC] Add support for pragma function

steplong added inline comments. Comment at: clang/lib/Parse/ParsePragma.cpp:3561 << "intrinsic"; return; } hans wrote: > since the above is just a warning, we should probably still call the ActOn.. > method? Hmm, I'm not sure because all the

[PATCH] D124702: [MSVC] Add support for pragma function

steplong updated this revision to Diff 427040. steplong added a comment. Changed std::vector to llvm::SmallVector Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124702/new/ https://reviews.llvm.org/D124702 Files:

[PATCH] D124702: [MSVC] Add support for pragma function

hans added a comment. >> And should we error/warn if the pragma occurs not in namespace scope? > > Oh I wasn't sure how to check if the function pragma was outside of a > function. In the ActOn.. methods I think you can check what the CurContext is. Comment at:

[PATCH] D124702: [MSVC] Add support for pragma function

steplong updated this revision to Diff 426998. steplong added a comment. Removed unnecessary braces (mentioned in the LLVM coding standard) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124702/new/ https://reviews.llvm.org/D124702 Files:

[PATCH] D124702: [MSVC] Add support for pragma function

steplong updated this revision to Diff 426802. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124702/new/ https://reviews.llvm.org/D124702 Files: clang/include/clang/Parse/Parser.h clang/include/clang/Sema/Sema.h

[PATCH] D124702: [MSVC] Add support for pragma function

steplong updated this revision to Diff 426702. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124702/new/ https://reviews.llvm.org/D124702 Files: clang/include/clang/Parse/Parser.h clang/include/clang/Sema/Sema.h

[PATCH] D124702: [MSVC] Add support for pragma function

steplong updated this revision to Diff 426433. steplong edited the summary of this revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124702/new/ https://reviews.llvm.org/D124702 Files: clang/include/clang/Parse/Parser.h

[PATCH] D124702: [MSVC] Add support for pragma function

steplong added a comment. In D124702#3485475 , @hans wrote: > From the MS docs: > >> Once a function pragma is seen, it takes effect at the first function >> definition that contains a specified intrinsic function. The effect >> continues to the end of

[PATCH] D124702: [MSVC] Add support for pragma function

hans added a comment. From the MS docs: > Once a function pragma is seen, it takes effect at the first function > definition that contains a specified intrinsic function. The effect continues > to the end of the source file, or to the appearance of an intrinsic pragma > specifying the same

[PATCH] D124702: [MSVC] Add support for pragma function

steplong created this revision. steplong added reviewers: rnk, rsmith, whunt, hans, thakis. Herald added a project: All. steplong requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. MSVC pragma function tells the compiler to generate calls to