This is an archive of the discontinued LLVM Phabricator instance.

Always allow "#pragma region".
ClosedPublic

Authored by mattd on Jan 18 2018, 9:59 AM.

Details

Summary

Both MS and PS4 targets are capable of recognizing the
existence of: #pragma region, #pragma endregion.

Since this pragma is only a hint for certain editors, and has no logic,
it seems helpful to permit this pragma in all cases, not just MS compatibility mode.

Diff Detail

Repository
rC Clang

Event Timeline

mattd created this revision.Jan 18 2018, 9:59 AM

Why not always support the pragma regardless of the compiler mode? Our "support" for it just ignores it anyway...

Why not always support the pragma regardless of the compiler mode? Our "support" for it just ignores it anyway...

Thanks for the reply @majnemer.

I am not opposed to that idea. My change just operates similar to the existing behavior. The only reservation I had against not always accepting the pragma is that it might mislead devs who are not using PS4/VS based environments.

Well, my understanding is that the pragma is a complete no-op even for MSVC, and is used only as a marker for editors such as Visual Studio's.
So, unconditionally ignoring it would seem to be fine.

mattd updated this revision to Diff 130523.Jan 18 2018, 5:18 PM
mattd retitled this revision from [LangOpts] Add a LangOpt to represent "#pragma region" support. to Always allow "#pragma region"..
mattd edited the summary of this revision. (Show Details)

I'm certainly fine with always allowing this pragma. I can see it useful for other editors that might wish to utilize the hint.

FWIW, I would also like this patch, because it would mean that I could build with -Werror even when the project includes headers written by MSVC-using people. Given that we know what "#pragma region" does, it hardly deserves an "unknown pragma" diagnostic! So this patch is great IMHO. :)

This revision is now accepted and ready to land.Jan 25 2018, 10:11 AM

Thanks @majnemer! Would you mind committing this on my behalf? I do not have commit access, thanks.

This revision was automatically updated to reflect the committed changes.