This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Handle non-function context before checking for diagnostic emission
ClosedPublic

Authored by pmrao on Feb 26 2021, 12:30 PM.

Details

Summary

Ensure that we are in a function declaration context before checking
the diagnostic emission status, to avoid dereferencing a NULL function
declaration.

Diff Detail

Event Timeline

pmrao created this revision.Feb 26 2021, 12:30 PM
pmrao requested review of this revision.Feb 26 2021, 12:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 26 2021, 12:30 PM
Herald added a subscriber: sstefan1. · View Herald Transcript
erichkeane added inline comments.Mar 1 2021, 6:43 AM
clang/lib/Sema/SemaOpenMP.cpp
1946

Tabs here instead of spaces.

I think this is the right fix for SYCL, but @bader should double-check that he's ok with this or OMP.

pmrao added inline comments.Mar 1 2021, 7:57 AM
clang/lib/Sema/SemaOpenMP.cpp
1946

Tabs here instead of spaces.

Are you suggesting that I change the spaces to tabs instead?

erichkeane added inline comments.Mar 1 2021, 7:59 AM
clang/lib/Sema/SemaOpenMP.cpp
1946

The reverse. Change the tabs you have at the beginning of the line to spaces. I believe that is what the double-arrow character in Phab represents (a tab).

pmrao marked an inline comment as not done.Mar 1 2021, 8:04 AM
pmrao added inline comments.
clang/lib/Sema/SemaOpenMP.cpp
1946

Oh! I misunderstood then. I thought phab was indicating that there are no changes in the line, except new indentation.

I didn't use tabs in my source file. I don't see it in the character set. Perhaps something introduced it during the upload. I will check.

erichkeane added inline comments.Mar 1 2021, 8:09 AM
clang/lib/Sema/SemaOpenMP.cpp
1946

Oh, it looks like you're right, that must be a new feature of Phab that looks shockingly like an old one :) I don't see tabs in the downloaded diff file, so consider that comment 'done'.

looks reasonable to me

bader accepted this revision.Mar 2 2021, 9:55 PM

LGTM.

This revision is now accepted and ready to land.Mar 2 2021, 9:55 PM