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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/SemaAttr.cpp:1070
+    SourceLocation Loc, const llvm::SmallSetVector<StringRef, 4> &Intrinsics) {
+  if (!CurContext->getRedeclContext()->isFileContext()) {
+    Diag(Loc, diag::err_pragma_intrinsic_function_scope);
----------------
steplong wrote:
> aaron.ballman wrote:
> > steplong wrote:
> > > aaron.ballman wrote:
> > > > What is `CurContext` when this gets called for your .c test file? I would have expected it to be the `TranslationUnitDecl` which should be a file context (getting the redecl context shouldn't do anything in the cases I've seen).
> > > It looks like it's a `FunctionDecl`
> > Wha? That seems rather surprising -- which function does it think the pragma is scoped to?
> > 
> > You might consider putting breakpoints in `PushDeclContext()` and `PopDeclContext()` to see what's going on (or search for other places where we assign to `CurContext` and break on those).
> This is what I see when I run it on pragma-ms-function.c:
> ```
> PUSHING TranslationUnit
> PUSHING Function foo1
> FOUND PRAGMA FUNCTION
> POPPING Function foo1
> PUSHING TranslationUnit
> PUSHING Function foo2
> FOUND PRAGMA FUNCTION
> POPPING Function foo2
> PUSHING TranslationUnit
> PUSHING Function foo3
> POPPING Function foo3
> PUSHING TranslationUnit
> ```
> I'm logging the swap in `PopDeclContext()` with POPPING and PUSHING and the push in `PushDeclContext()` with just PUSHING.
> I'm also logging FOUND PRAGMA in the pragma handler
Huh. For fun, can you try changing the test to:
```
void foo1(char *s, char *d, size_t n) {
  bar(s);
  memset(s, 0, n);
  memcpy(d, s, n);
}

int i; // Now there's a declaration here

#pragma function(strlen, memset)
```
and see if you get different results? I'm wondering if what's happening is that the `CurContext` is being updated after we've lexed the next token from the preprocessor, which means we're still in the context of `foo1` until after we've processed the pragma due to it being a preprocessing token. It still wouldn't make much sense to me, because I think we should hit that on the `}` for `foo1()`, but it's a shot in the dark.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124702