[PATCH] D91129: Print source location in the error message when parens are missing around sizeof typename and the expression is inside macro expansion

2020-12-15 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 marked an inline comment as done. shivanshu3 added inline comments. Comment at: clang/lib/Parse/ParseExpr.cpp:2267-2280 +SourceLocation OpTokLoc = OpTok.getLocation(); +if (OpTokLoc.isMacroID()) { + SourceLocation OpTokExpansionLoc = +

[PATCH] D91129: Print source location in the error message when parens are missing around sizeof typename and the expression is inside macro expansion

2020-12-15 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 updated this revision to Diff 312055. shivanshu3 added a comment. Don't assume that `getLocForEndOfToken` will fail if and only if the token is a macro Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91129/new/

[PATCH] D91659: Allow enum typedefs to be referenced with the 'enum' specifier under MSVC compat mode

2020-12-15 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 added a comment. Ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91659/new/ https://reviews.llvm.org/D91659 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D91129: Print source location in the error message when parens are missing around sizeof typename and the expression is inside macro expansion

2020-12-15 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 added a comment. Ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91129/new/ https://reviews.llvm.org/D91129 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D91129: Print source location in the error message when parens are missing around sizeof typename and the expression is inside macro expansion

2020-12-01 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 updated this revision to Diff 308827. shivanshu3 added a comment. Clang format the test file. Forgot to do it before. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91129/new/ https://reviews.llvm.org/D91129 Files:

[PATCH] D91659: Allow enum typedefs to be referenced with the 'enum' specifier under MSVC compat mode

2020-12-01 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:15752-15754 + bool AnonymousEnumEligible = getLangOpts().MSVCCompat && + (Kind == TTK_Enum) && +

[PATCH] D91659: Allow enum typedefs to be referenced with the 'enum' specifier under MSVC compat mode

2020-12-01 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 updated this revision to Diff 308803. shivanshu3 retitled this revision from "Allow anonymous enum typedefs to be referenced with the 'enum' specifier under MSVC compat mode" to "Allow enum typedefs to be referenced with the 'enum' specifier under MSVC compat mode". shivanshu3 edited

[PATCH] D91659: Allow anonymous enum typedefs to be referenced with the 'enum' specifier under MSVC compat mode

2020-11-17 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 marked 3 inline comments as done. shivanshu3 added a comment. Thank you very much for the review @rsmith! Comment at: clang/lib/Sema/SemaDecl.cpp:15752-15754 + bool AnonymousEnumEligible = getLangOpts().MSVCCompat && +

[PATCH] D91659: Allow anonymous enum typedefs to be referenced with the 'enum' specifier under MSVC compat mode

2020-11-17 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 updated this revision to Diff 305971. shivanshu3 added a reviewer: rsmith. shivanshu3 added a comment. Addressing some of rsmith's comments: - Add a test case where an enum variable is declared in the same scope as the enum definition - Fix a style issue with parentheses - Simplify

[PATCH] D91659: Allow anonymous enum typedefs to be referenced with the 'enum' specifier under MSVC compat mode

2020-11-17 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 created this revision. shivanshu3 added reviewers: zahen, tiagoma, rnk, hans. Herald added a project: clang. Herald added a subscriber: cfe-commits. shivanshu3 requested review of this revision. Goal: Clang should be able to parse the following code with '-fms-compatibility' because

[PATCH] D91129: Print source location in the error message when parens are missing around sizeof typename and the expression is inside macro expansion

2020-11-12 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 updated this revision to Diff 304973. shivanshu3 edited the summary of this revision. shivanshu3 added a comment. Use getFileLoc instead of getExpansionLoc to get the location of error. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D91129: Print source location in the error message when parens are missing around sizeof typename and the expression is inside macro expansion

2020-11-10 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 added a reviewer: tiagoma. shivanshu3 added a comment. Note that this regression seems to be there ever since Clang 3.4.1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91129/new/ https://reviews.llvm.org/D91129

[PATCH] D91129: Print source location in the error message when parens are missing around sizeof typename and the expression is inside macro expansion

2020-11-09 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 created this revision. shivanshu3 added reviewers: zahen, hans, rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. shivanshu3 requested review of this revision. Given the following code: void Foo(int); #define Bar(x) Foo(x) void Baz() {

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

2020-10-06 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 updated this revision to Diff 296393. shivanshu3 added a comment. Address Hans' comments - Prepend %s with "--" for clang-cl - Remove a redundant test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88680/new/

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

2020-10-05 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 updated this revision to Diff 296340. shivanshu3 added a comment. Address Reid's comments: - Use hasFlag instead of hasArg - Add test cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88680/new/ https://reviews.llvm.org/D88680

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

2020-10-02 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 added a comment. Also, I wanted to mention that clang-cl.exe actually crashes deterministically when running precompiles for 2 of our headers when using '-fpch-instantiate-templates' with the following signature: 1. parser at end of file 2. Per-file LLVM IR generation

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

2020-10-02 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 added a comment. > Can you get at least some testcase, even if not small? Yes, in a few days I can get you the test case. I'll have to figure out what our company policy is for sharing code and if I should obfuscate it so I don't get into trouble :) > MSVC does some kind of

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

2020-10-01 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 added a comment. In D88680#2307564 , @rnk wrote: > I think the flag was originally intended to be an internal -cc1 flag not > exposed to users. You should be able to work around your problem with > `-Xclang -fno-pch-instantiate-templates`,

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

2020-10-01 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 added a comment. Note that I do not have commit access and this change will have to be committed by someone else on my behalf. Please use the following commit author details: "Shivan Goyal " Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

2020-10-01 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 created this revision. shivanshu3 added reviewers: llunak, rnk, rsmith, hans, zahen. shivanshu3 added a project: clang. Herald added subscribers: cfe-commits, dang. shivanshu3 requested review of this revision. A lot of our code building with clang-cl.exe using Clang 11 was failing

[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-03 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 marked an inline comment as done. shivanshu3 added a comment. Note that I do not have commit access and this change will have to be committed by someone else on my behalf. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86999/new/

[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-03 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 marked 2 inline comments as done. shivanshu3 added inline comments. Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:117 + // do not remove those when using the cl driver. + bool IsDependencyFileArg; + if (Arg.startswith("/showIncludes") ||

[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-03 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 updated this revision to Diff 289793. shivanshu3 added a comment. - Remove the bool `IsDependencyFileArg` in the implementation of `getClangStripDependencyFileAdjuster()` to make it simpler. - Add an extra argument after -MT in the test case to ensure we do not strip arguments after

[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-02 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 marked 5 inline comments as done. shivanshu3 added inline comments. Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:117 + // do not remove those when using the cl driver. + bool IsDependencyFileArg; + if (Arg.startswith("/showIncludes") ||

[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-02 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 updated this revision to Diff 289606. shivanshu3 added a comment. - Simplified the implementation of `getDriverMode` and got rid of the `Optional` return type. - When using the cl driver mode, we do not want to skip the next argument for -MF, -MT, -MQ. Repository: rG LLVM Github

[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-01 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 created this revision. shivanshu3 added reviewers: aeubanks, kadircet. Herald added a project: clang. Herald added a subscriber: cfe-commits. shivanshu3 requested review of this revision. Herald added a subscriber: ormris. MSVC's cl.exe has a few command line arguments which start with