AaronBallman wrote:

> > Thank you for working on this! Please be sure to add a release note to 
> > `clang/docs/ReleaseNotes.rst` so users know about the new functionality too.
> 
> @AaronBallman Thanks for the review. I added a point to the "Attribute 
> Changes" section - not sure if this is the right place since `[[msvc::` 
> attributes are rarely mentioned in the previous release notes.

That's a good place for it!

> > Are there plans to also handle `[[msvc::forceinline_calls]]` 
> > (https://learn.microsoft.com/en-us/cpp/cpp/attributes?view=msvc-170#msvcforceinline_calls)
> >  which sounds like our `[[clang::always_inline]]` behavior on statements?
> 
> I am aware of `[[msvc::forceinline_calls]]`. Initially I wanted to keep the 
> PR simple, so I let `[[msvc::forceinline]]` have an "unspecified" effect when 
> added to statements, as MS documentation did not specify what would happen in 
> that case.

This is interesting; it seems cl silently accepts the attribute on 
non-declaration statements, but diagnoses any declaration other than a function 
declaration as `attribute [[msvc::forceinline]] may only be applied to a 
function declaration`: https://godbolt.org/z/daG9nr1qc

But it doesn't have any effect on the call behavior, at least that I can find: 
https://godbolt.org/z/cr9TKcx5M

We'd prefer to diagnose the attribute as being explicitly ignored so users are 
not confused (that's our usual position on all ignored attributes). But because 
Microsoft accepts, I think this is a bug that should be [reported to 
Microsoft](https://developercommunity.visualstudio.com/VisualStudio/) to see if 
they'll change behavior. Things get harder for users if we accept code 
Microsoft rejects or we reject code Microsoft accepts, so we try to honor the 
vendor's extensions.

> Now, with all the additional diagnostics requirements, supporting 
> `[[msvc::forceinline_calls]]` comes at a relatively low cost, so I added that.

Thanks!


https://github.com/llvm/llvm-project/pull/185282
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to