[PATCH] D132503: Add cxx scope if needed for requires clause.

2022-08-25 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: clang/lib/Parse/ParseTemplate.cpp:293 +if (Tok.is(tok::kw_requires)) { + CXXScopeSpec ScopeSpec = DeclaratorInfo.getCXXScopeSpec(); + DeclaratorScopeObj DeclScopeObj(*this, ScopeSpec); Not sure about thi

[PATCH] D132503: Add cxx scope if needed for requires clause.

2022-08-25 Thread Luke Nihlen via Phabricator via cfe-commits
luken-google marked 2 inline comments as done. luken-google added a comment. Thanks for the feedback and the code review. I've uploaded a new revision with the requested changes, PTAL. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132503/new/ http

[PATCH] D132503: Add cxx scope if needed for requires clause.

2022-08-25 Thread Luke Nihlen via Phabricator via cfe-commits
luken-google updated this revision to Diff 455620. luken-google added a comment. Responding to feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132503/new/ https://reviews.llvm.org/D132503 Files: clang/docs/ReleaseNotes.rst clang/lib/Pa

[PATCH] D132503: Add cxx scope if needed for requires clause.

2022-08-25 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Like @ilya-biryukov , I don't really know enough of the uses of 'DeclaratorScopeObj ' to know if that is right, but other than Aaron's suggestions, I don't see anything to suggest. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D132503: Add cxx scope if needed for requires clause.

2022-08-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Also, don't forget to add a release note since this is fixing an issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132503/new/ https://reviews.llvm.org/D132503 ___ cfe-c

[PATCH] D132503: Add cxx scope if needed for requires clause.

2022-08-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: erichkeane, clang-language-wg. aaron.ballman added a comment. FWIW, I think this looks correct as well. I've added some reviewers just in case there's something I've missed regarding concepts. Comment at: clang/lib/Parse/ParseTemplate.cpp:293-301

[PATCH] D132503: Add cxx scope if needed for requires clause.

2022-08-25 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a subscriber: aaron.ballman. ilya-biryukov added a comment. Thanks for the fix. This looks ok to me, except that I am a bit suspicious of the fact that `DeclaratorScopeObj` is used somewhat rarely. I suspect we might want a different guard class for this, e.g. something simil

[PATCH] D132503: Add cxx scope if needed for requires clause.

2022-08-24 Thread Luke Nihlen via Phabricator via cfe-commits
luken-google added a comment. Note that the following tests: Builtins-i386-linux :: muldc3_test.c SanitizerCommon-asan-i386-Linux :: Linux/crypt_r.cpp SanitizerCommon-asan-i386-Linux :: Posix/crypt.cpp SanitizerCommon-lsan-i386-Linux :: Linux/crypt_r.cpp SanitizerCommon-lsan-i386-Linux

[PATCH] D132503: Add cxx scope if needed for requires clause.

2022-08-24 Thread Luke Nihlen via Phabricator via cfe-commits
luken-google updated this revision to Diff 455217. luken-google added a comment. Fix clang-format errors Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132503/new/ https://reviews.llvm.org/D132503 Files: clang/lib/Parse/ParseTemplate.cpp clang/

[PATCH] D132503: Add cxx scope if needed for requires clause.

2022-08-23 Thread Luke Nihlen via Phabricator via cfe-commits
luken-google created this revision. Herald added a project: All. luken-google requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes issue #55216. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D132503 Files: clang/lib/