[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2018-12-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. I think the ideal solution would be something like: If a variable produced by a macro expansion shadows a variable not produced by a macro expansion, do not warn immediately. Instead, warn if we see a use of the inner variable in a context that is not produced by the sam

Re: [PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-31 Thread Roman Lebedev via cfe-commits
On Mon, Jul 31, 2017 at 9:57 PM, David Blaikie wrote: > > > On Tue, Jul 25, 2017 at 1:19 AM Roman Lebedev via Phabricator via > cfe-commits wrote: >> >> lebedev.ri added a comment. >> >> How does this relate to the gcc behavior? >> I suspect not everyone would want to have this relaxed `-Wshadow`

Re: [PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-31 Thread David Blaikie via cfe-commits
On Tue, Jul 25, 2017 at 1:19 AM Roman Lebedev via Phabricator via cfe-commits wrote: > lebedev.ri added a comment. > > How does this relate to the gcc behavior? > I suspect not everyone would want to have this relaxed `-Wshadow` mode. > Generally I think the goal hasn't been to allow a clang use

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. How does this relate to the gcc behavior? I suspect not everyone would want to have this relaxed `-Wshadow` mode. Perhaps it could be hidden under some new flag, which is not going to be automatically enabled by `-Weverything`. Like, `-Wshadow-in-macros` which does not

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-24 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. Yes, your example would work, too. The specific use case I have is where we are shadowing global variables. https://reviews.llvm.org/D35783 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-b

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-24 Thread Kim Gräsman via Phabricator via cfe-commits
kimgr added a comment. Any chance you could try my example too? It seems like this approach might catch it. But does the new approach really do anything different for your original example? https://reviews.llvm.org/D35783 ___ cfe-commits mailing

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-24 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. Like this? https://reviews.llvm.org/D35783 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-24 Thread Daniel Jasper via Phabricator via cfe-commits
djasper updated this revision to Diff 107885. djasper added a comment. Updated to be a bit more strict (warn if declarations from the same context get shadowed). https://reviews.llvm.org/D35783 Files: lib/Sema/SemaDecl.cpp test/SemaCXX/warn-shadow.cpp Index: test/SemaCXX/warn-shadow.cpp =

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-24 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added a comment. Could we perhaps only suppress the warning when we shadow a variable within the same declcontext? Generally, with macros, shadowing local variables is more surprising. https://reviews.llvm.org/D35783 ___ cfe-commits mailing

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-24 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. I don't understand how my patch description says that. I am trying to be very explicit about that fact that it doesn't, see last paragraph. I have thought about what you are suggesting, but - A DeclContext doesn't have getEndLoc(). - The DeclContext NewDC here is usuall

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-24 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added a comment. Your patch description sounds like you're only switching the warning off when the scope ends in a macro, but the patch looks different. For example: #define M int x int x; void f() { M; x = 42; // the user probably meant ::x here } would also be suppre

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-23 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. I'd be ok, with missing that case, but I am happy to hear more opinions. https://reviews.llvm.org/D35783 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-23 Thread Kim Gräsman via Phabricator via cfe-commits
kimgr added a comment. I've seen this in the wild: #define LOG(m) \ { \ std::ostringstream os; \ os << m << "\n"; \ LogWrite(os.str()); \ } auto os = GetOSName(); LOG("The OS is " << os); It looks like your patch would miss this case. Not sure if

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-23 Thread Daniel Jasper via Phabricator via cfe-commits
djasper created this revision. The case, I am particularly interested in is: #define A(x) \ ... \ if (...) { \ int SomeVariable = 1; \ ...; \ } Here, SomeVariable never leaves the scope of the macro and at t