New user self-registration is disabled due to spam. For an account please email bugs-admin@lists.llvm.org with your e-mail address and full name.

Bug 42193 - hasName AST matcher is confused by extern "C" in namespace
Summary: hasName AST matcher is confused by extern "C" in namespace
Status: RESOLVED FIXED
Alias: None
Product: clang
Classification: Unclassified
Component: Frontend (show other bugs)
Version: unspecified
Hardware: PC Windows NT
: P normal
Assignee: Nathan James
URL:
Keywords:
Depends on:
Blocks: release-10.0.0
  Show dependency tree
 
Reported: 2019-06-08 02:27 PDT by Patrick Reisert
Modified: 2020-02-27 04:13 PST (History)
5 users (show)

See Also:
Fixed By Commit(s): rG16cabf278fc8c14d415e677ce0bc40d46a6de30d


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Reisert 2019-06-08 02:27:31 PDT
Given the following code:

namespace foo {
    extern "C" void test() {}
}


... I expect an AST matcher declared as functionDecl(hasName("::foo::test")) to match the function, but functionDecl(hasName("::test")) should not match. However, the opposite is the case. (see the example on Compiler Explorer: http://ce.steveire.com/z/VDTi_g)

When I do this locally with a build with assertions, I get an assertion failure at https://github.com/llvm/llvm-project/blob/bca56ab073a00cdec8e0995e3119baf25cf775b8/clang/lib/ASTMatchers/ASTMatchersInternal.cpp#L540, so I think the problem is that matchesNodeFullFast does not correctly handle extern "C" functions.
Comment 1 Nathan James 2020-02-26 11:18:22 PST
Got a patch in the work https://reviews.llvm.org/D75202. Added bonus it doesn't fire off those assertions too.
Comment 2 Nathan James 2020-02-26 14:13:02 PST
Fixed in https://reviews.llvm.org/rG16cabf278fc8c14d415e677ce0bc40d46a6de30d

As this causes a crash I'll mark it as a release blocker, probably wont make it in time though
Comment 3 Hans Wennborg 2020-02-27 04:13:54 PST
(In reply to Nathan James from comment #2)
> Fixed in https://reviews.llvm.org/rG16cabf278fc8c14d415e677ce0bc40d46a6de30d
> 
> As this causes a crash I'll mark it as a release blocker, probably wont make
> it in time though

It looks like a very safe change though. Pushed to 10.x as 8f2858eb07085e13544316abfd3c0e90cef3eb93.