1501821 - Update clang-plugin code to use new names getBeginLoc/getEndLoc

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, enhancement)

Description

[(Away)]

Some of the AST functions that we use have been renamed upstream [1] and as of r341573 the old names no longer exist [2].

We'll need to fix up the names when updating to clang trunk or 8.0.0. This may be particularly hairy now that static analysis is incorporated into our primary 
builds: is clang-plugin built by more than one clang version?

[1] http://lists.llvm.org/pipermail/cfe-dev/2018-August/058853.html
[2] https://github.com/llvm-mirror/clang/commit/cdacbfee79e73cda6a3134cafef3aa7c599da21b

Comment 1

[Andi [:andi]]

Let's make sure we don't regress on static-analysis linux64 build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7fc2fdf3a715acead0643f928216156c5270c326

Comment 2

[Jeff Walden [:Waldo]]

Attachment: Implement GetBeginLoc and GetEndLoc helper functions in the clang plugin that use SFINAE to pick the function from getBeginLoc/getLocStart and getEndLoc/getLocEnd that clang actually implements

Hi!  I just tried to follow my usual practice of compiling clang tip and using that (having been forced into it by the new clang 7 requirement) and ran into this.  This patch theoretically *should* allow clang-plugin to work with code before *or* after these functions were renamed.  It does seem to compile with clang tip for me, but I run into bug 1495511 so I can't fully test.

Comment 3

[(Away)]

Comment on attachment 9022323 [details] [diff] [review]
Implement GetBeginLoc and GetEndLoc helper functions in the clang plugin that use SFINAE to pick the function from getBeginLoc/getLocStart and getEndLoc/getLocEnd that clang actually implements

This seems like an awful lot of gymnastics for what needs to be achieved here. I would have been perfectly happy to see Andi's patch from comment 1 land. In the long term we need to use get{Begin,End}Loc exclusively. The simple #define for short-term backwards compatibility (that we can trivially rip out when ready) seemed like a more proportionate solution to me.

Comment 4

[Jeff Walden [:Waldo]]

IMO redefining stuff is a bit more scattershot than doing the explicitly correct thing, but do whatever -- just, it'd be nice if it were done sooner rather than later so I don't have to cart around a local patch to keep building with the plugin in play.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 9022323 [details] [diff] [review]
Implement GetBeginLoc and GetEndLoc helper functions in the clang plugin that use SFINAE to pick the function from getBeginLoc/getLocStart and getEndLoc/getLocEnd that clang actually implements

Review of attachment 9022323 [details] [diff] [review]:
-----------------------------------------------------------------

MozsearchIndexer.cpp needs to work without pulling in files from the parent dir, as we have a copy of that folder building standalone elsewhere that we keep in sync. So for that file at least I'd prefer an #ifdef solution.

Comment 6

[Andi [:andi]]

Attachment: Bug 1501821 - Update clang-plugin in order to make it compatiable with clang 8.0.0 trunk. r?froydnj

Comment 7

[Andi [:andi]]

Jeff didnt mean to steal your patch, I’ve alresdy wrote the Fox for this when I’ve pushed to try but forgot to assign it to myself.

Comment 8

[Andi [:andi]]

Since the patch has been accepted let's run again try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=09b89c1684c660cf9496e5765ca6ef0ebdd97429

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/538a16d49514
Update clang-plugin in order to make it compatiable with clang 8.0.0 trunk. r=froydnj

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/538a16d49514