Page MenuHomePhabricator

[analyzer][CrossTU][NFC] Generalize to external definitions instead of external functions
ClosedPublic

Authored by r.stahl on Jan 8 2019, 7:05 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

r.stahl created this revision.Jan 8 2019, 7:05 AM
xazax.hun accepted this revision.Jan 8 2019, 7:15 AM

Some nits inline. Otherwise looks good to me.

include/clang/CrossTU/CrossTranslationUnit.h
32 ↗(On Diff #180660)

Unrelated change?

106 ↗(On Diff #180660)

These comment changes might better be in the next patch.

This revision is now accepted and ready to land.Jan 8 2019, 7:15 AM

Thanks for the quick response! Will wait a couple days to see if someone else has something to add.

But I think something is wrong here...

When trying to "arc diff" I got the following error:

 Exception 
Command failed with error #1!
COMMAND
svn propget 'svn:mime-type' '/data/work/commitllvm/llvm/tools/clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp'@

STDOUT
(empty)

STDERR
svn: warning: W200017: Property 'svn:mime-type' not found on '/data/work/commitllvm/llvm/tools/clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp@'
svn: E200000: A problem occurred; see other errors for details

So I did:

svn propset svn:mime-type text/plain tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

This was on all the new/renamed files.

Now this diff shows some weird "svn:mime-type" changes which are probably unwanted. But only on two of the new files (?).

I only found this online, but weird that no one else is having this issue: https://secure.phabricator.com/T10608

martong accepted this revision.Jan 8 2019, 7:55 AM

Looks good to me (but xazax's comments are valid)! Thank you.

r.stahl updated this revision to Diff 181073.Jan 10 2019, 8:36 AM

addressed review comments

This revision was automatically updated to reflect the committed changes.
NoQ added a comment.Jan 10 2019, 3:12 PM

Hmm, this fails for me. The substitution fails for some reason:

FAIL: Clang :: Analysis/func-mapping-test.cpp (211 of 569)
******************** TEST 'Clang :: Analysis/func-mapping-test.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   %clang_extdef_map /Users/adergachev/svn/llvm/tools/clang/test/Analysis/func-mapping-test.cpp -- | /Users/adergachev/svn/release/bin/FileCheck /Users/adergachev/svn/llvm/tools/clang/test/Analysis/func-mapping-test.cpp
--
Exit Code: 2

Command Output (stderr):
--
/Users/adergachev/svn/release/tools/clang/test/Analysis/Output/func-mapping-test.cpp.script: line 1: %clang_extdef_map: command not found
FileCheck error: '-' is empty.
FileCheck command line:  /Users/adergachev/svn/release/bin/FileCheck /Users/adergachev/svn/llvm/tools/clang/test/Analysis/func-mapping-test.cpp

--

********************
Testing Time: 5.12s
********************
Failing Tests (1):
    Clang :: Analysis/func-mapping-test.cpp

I'm not really good with LIT and i don't immediately see any problems here. If it only fails for me, please let me know so that i could see if the problem is on my end or otherwise help investigating the problem :)

NoQ added a comment.Jan 10 2019, 6:41 PM

Hmm, it disappeared after a clean rebuild. I think the problem was that i did make clang and then ran tests manually, instead of make, so the tool was not rebuilt and was only present under its old name. Because the tool does get rebuilt under make check-clang-analysis, i guess i was just doing an unsupported thing.

Sry for distraction, pls nvm :)