[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

2022-05-12 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added inline comments. Comment at: clang/test/Sema/warn-strict-prototypes.m:20 + // know it's a block when diagnosing. + void (^block2)(void) = ^void() { // expected-warning {{a function declaration without a prototype is deprecated in all versions of C}} };

[PATCH] D125488: [Preprocessor] Make the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer`

2022-05-12 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D125488#3510265 , @akyrtzi wrote: > In D125488#3510214 , @dexonsmith > wrote: > >> Is there code in DepFS that can/should be deleted as part of this patch, or >> in a follow-up, or

[PATCH] D125488: [Preprocessor] Make the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer`

2022-05-12 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D125488#3510320 , @akyrtzi wrote: > In D125488#3510297 , @dexonsmith > wrote: > >> [To be clear, my question was because I don't see this patch deleting the >> code path that minim

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

2022-05-13 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a subscriber: arphaman. dexonsmith added a comment. In D122895#3511376 , @aaron.ballman wrote: > In D122895#3511312 , @aaron.ballman > wrote: > >> However, I think the blocks behavior is a bug b

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

2022-05-13 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D122895#3511611 , @dexonsmith wrote: > Previously, `-Wstrict-prototypes` was useful for preventing actual bugs in > code. For example, it's important to have a warning that catches code like this: void f1(void (^block)

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

2022-05-13 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D122895#3511649 , @aaron.ballman wrote: > In D122895#3511611 , @dexonsmith > wrote: > >> Sure, I'm all for adding a new warning for users that want a pedantic >> warning. Can it b

[PATCH] D124974: [clang] Include clang config.h in LangStandards.cpp

2022-05-13 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. LGTM, thanks! One comment online below. Comment at: clang/include/clang/Config/config.h.cmake:19 #cmakedefine CLANG_DEFAULT_STD_C LangStandard::lang_${CLANG_DEFAULT_STD_C} +// Always #define something so that missi

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

2022-05-17 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D122895#3511855 , @aaron.ballman wrote: >> (It also seems unfortunate to regress the false positive rate of this >> diagnostic before `-fstrict-prototypes` is available.) > > `-fno-knr-functions` is available already today

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

2022-05-21 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added inline comments. Comment at: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp:175 "#define MACRO con tent ", Out)); - EXPECT_STREQ("#define MACRO con tent\n", Out.data()); + EXPECT_STREQ("#define MACRO con tent\n", Out.data()); ---

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

2022-05-21 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added inline comments. Comment at: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp:175 "#define MACRO con tent ", Out)); - EXPECT_STREQ("#define MACRO con tent\n", Out.data()); + EXPECT_STREQ("#define MACRO con tent\n", Out.data()); ---

[PATCH] D66511: [clang-scan-deps] Skip UTF-8 BOM in source minimizer

2019-08-23 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added inline comments. Comment at: lib/Lex/DependencyDirectivesSourceMinimizer.cpp:822 bool Minimizer::minimizeImpl(const char *First, const char *const End) { + skipUTF8ByteOrderMark(First, End); while (First != End) Is skipping this the right th

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith created this revision. dexonsmith added a reviewer: arphaman. Herald added a subscriber: ributzka. dexonsmith added a comment. @arphaman, is there a reason you think `ErrorOr` is more appropriate long-term here? `FileManager::getFileRef` is a modern API which we expect to convert to

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. @arphaman, is there a reason you think `ErrorOr` is more appropriate long-term here? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66705/new/ https://reviews.llvm.org/D66705 ___ cfe-commits mailing list cfe-comm

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith updated this revision to Diff 217024. dexonsmith edited the summary of this revision. dexonsmith added a subscriber: lhames. dexonsmith added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Updated patch after running `check-clang` and learning more abo

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith marked 2 inline comments as done. dexonsmith added inline comments. Comment at: clang/lib/Lex/HeaderSearch.cpp:1079 if (!File) { // Check ".../Frameworks/HIToolbox.framework/PrivateHeaders/HIToolbox.h" HeadersFilename = FrameworkName; arph

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith updated this revision to Diff 217025. dexonsmith marked an inline comment as done. dexonsmith edited the summary of this revision. dexonsmith added a comment. Also update `FileEntryRef` to be copy- and move-assignable so that `Optional` can be assigned to. CHANGES SINCE LAST ACTION

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith updated this revision to Diff 217026. dexonsmith edited the summary of this revision. dexonsmith added a comment. Fix the change to CompilerInstance.cpp. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66705/new/ https://reviews.llvm.org/D66705 Files: clang/include/clang/Bas

[PATCH] D66710: ASTReader: Bypass overridden files when reading PCHs

2019-08-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith created this revision. dexonsmith added reviewers: rsmith, arphaman, akyrtzi, bruno. Herald added a subscriber: ributzka. If contents of a file that is part of a PCM are overridden when reading it, but weren't overridden when the PCM was being built, the ASTReader will emit an error. N

[PATCH] D66713: ContentCache: Drop getBuffer's dependency on SourceManager

2019-08-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith created this revision. dexonsmith added reviewers: arphaman, bruno. Herald added a subscriber: ributzka. Refactor ContentCache::IsSystemFile to IsFileVolatile, checking SourceManager::userFilesAreVolatile at construction time. This is a step toward lowering ContentCache down from Sourc

[PATCH] D66713: ContentCache: Drop getBuffer's dependency on SourceManager

2019-08-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith updated this revision to Diff 217036. dexonsmith added a comment. (Updating the diff with full context.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66713/new/ https://reviews.llvm.org/D66713 Files: clang/include/clang/Basic/SourceManager.h clang/lib/Basic/SourceManage

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-26 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith closed this revision. dexonsmith added a comment. r369943. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66705/new/ https://reviews.llvm.org/D66705 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D66710: ASTReader: Bypass overridden files when reading PCHs

2019-08-26 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith updated this revision to Diff 217229. dexonsmith added a comment. New diff with full context. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66710/new/ https://reviews.llvm.org/D66710 Files: clang/include/clang/Basic/FileManager.h clang/include/clang/Basic/SourceManager.h

[PATCH] D66713: ContentCache: Drop getBuffer's dependency on SourceManager

2019-08-26 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith closed this revision. dexonsmith added a comment. Committed in r369958. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66713/new/ https://reviews.llvm.org/D66713 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://list

[PATCH] D66782: SourceManager: Prefer Optional over MemoryBuffer*

2019-08-26 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith created this revision. dexonsmith added reviewers: arphaman, Bigcheese, rsmith. Herald added subscribers: llvm-commits, ributzka, kadircet, jkorous, hiraditya. Herald added a reviewer: martong. Herald added a reviewer: shafik. Herald added a project: LLVM. Change the APIs in SourceManag

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

2019-08-27 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D66556#1646229 , @aganea wrote: > This failed the build - I've changed `unix2dos` to `svn:eol-style CRLF` > instead. Can we count on `tr` when we have `shell`? If so, I suggest this instead: // REQUIRES: shell // RUN

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

2019-08-27 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D66556#1647762 , @dexonsmith wrote: > In D66556#1646229 , @aganea wrote: > > > This failed the build - I've changed `unix2dos` to `svn:eol-style CRLF` > > instead. > > > Can we count

[PATCH] D58497: Clear the KnownModules cache if the preprocessor is going away

2019-08-27 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. Herald added a subscriber: wuzish. Is this still a problem? I'd be interested to understand *why* the preprocessor is going away here. Is the `CompilerInstance` being reused for two different compilations? Is that what we should fix instead? Repository: rC Clan

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

2019-08-27 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D66556#1648109 , @rnk wrote: > I'm not sure what happens, but I see you added .gitattributes. I'd commit it > as is. Buildbots using svn will keep working. You can check that the monorepo > has the right line endings afterw

[PATCH] D66907: [Modules] Fix rebuilding an updated module for each of its consumers.

2019-08-28 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. This LGTM. The key insight you're making is that when you're importing B into A and B's signature doesn't match what A expects, that does not mean that B is out-of-date. B may have j

[PATCH] D66710: ASTReader: Bypass overridden files when reading PCHs

2019-08-28 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith marked 2 inline comments as done. dexonsmith added inline comments. Comment at: clang/include/clang/Basic/FileManager.h:320 + /// twice, you get two new file entries. + const FileEntry *getBypassFile(const FileEntry &VFE); + bruno wrote: > Does it ma

[PATCH] D66710: ASTReader: Bypass overridden files when reading PCHs

2019-08-28 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith updated this revision to Diff 217754. dexonsmith added a comment. Added a FileManagerTest and changed FileManager::getBypassFile to use FileEntryRef. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66710/new/ https://reviews.llvm.org/D66710 Files: clang/include/clang/Basic/

[PATCH] D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes

2019-08-28 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a reviewer: erik.pilkington. dexonsmith added a comment. This could cause a lot of churn in existing projects (especially with `static void foo()`), without giving Clang any new information. I'm wary of this. > Zero-parameter K&R definitions specify that the function has no >

[PATCH] D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes

2019-08-29 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D66919#1650775 , @aaron.ballman wrote: > In D66919#1650174 , @dexonsmith > wrote: > > > This could cause a lot of churn in existing projects (especially with > > `static void foo()`

[PATCH] D66989: FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC

2019-08-29 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith created this revision. dexonsmith added reviewers: arphaman, Bigcheese, jkorous. Herald added a subscriber: ributzka. Remove this dead code. We always close the file. https://reviews.llvm.org/D66989 Files: clang/include/clang/Basic/FileManager.h clang/lib/Basic/FileManager.cpp

[PATCH] D66989: FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC

2019-08-30 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith closed this revision. dexonsmith added a comment. r370488 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66989/new/ https://reviews.llvm.org/D66989 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes

2019-08-30 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. > In D66919#1650174 , @dexonsmith > wrote: > >> We could carve out a `-W` flag (if it doesn't already exist) that warns if >> you incorrectly pass parameters to a function whose definition has no >> prototype > > > The warn

[PATCH] D66710: ASTReader: Bypass overridden files when reading PCHs

2019-08-30 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith closed this revision. dexonsmith marked an inline comment as done. dexonsmith added a comment. r370546. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66710/new/ https://reviews.llvm.org/D66710 ___ cfe-commits mailing list cfe-com

[PATCH] D67026: Introduce a DirectoryEntryRef that stores both a reference and an accessed name to the directory entry

2019-08-30 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. A few minor comments. Assuming the `FIXME` I point out was intentionally left for later, this LGTM. Comment at: clang/include/clang/Basic/FileManager.h:59 +public:

[PATCH] D67029: SourceManager: Prefer Optional over MemoryBuffer*

2019-08-30 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith created this revision. dexonsmith added reviewers: arphaman, Bigcheese. Herald added subscribers: llvm-commits, ributzka, kadircet, jkorous, hiraditya. Herald added a reviewer: martong. Herald added a reviewer: shafik. Herald added a project: LLVM. Change the APIs in SourceManager retur

[PATCH] D67030: ContentCache: Simplify by always owning the MemoryBuffer

2019-08-30 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith created this revision. dexonsmith added reviewers: arphaman, Bigcheese, rsmith. Herald added a subscriber: ributzka. This changes ContentCache::Buffer to use `std::unique_ptr` instead of the PointerIntPair. It drops the (mostly unused) `DoNotFree` bit in favour of creating a new, non-o

[PATCH] D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes

2019-09-02 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. I went back to read notes from when we deployed `-Wstrict-prototypes` (which we have had on-by-default for our users for a couple of years, since it catches lots of `-fblocks`-related bugs). I was mainly objecting because I had (mis)remembered trying and backing out

[PATCH] D58497: Clear the KnownModules cache if the preprocessor is going away

2019-09-05 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. I don't think you need prior frontend expertise, just some time and patience since the modules code has some technical debt. If you are still willing to look at it, I agree with Richard's suggestion that we could merge this with the map in the Module Manager. One a

[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

2019-09-05 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added inline comments. Comment at: clang/lib/Lex/HeaderSearch.cpp:888 bool InUserSpecifiedSystemFramework = false; -bool HasBeenMapped = false; +bool CurrentInHeaderMap = false; bool IsFrameworkFoundInDir = false; Why not name this th

[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

2019-09-06 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM. I have one idea for you to consider inline. Comment at: clang/lib/Lex/HeaderSearch.cpp:892-902 +IsInHeaderMap, MappedName); +if (!MappedName.empty(

[PATCH] D36427: [libcxxabi][demangler] Improve representation of substitutions/templates

2017-08-07 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith requested changes to this revision. dexonsmith added a comment. This revision now requires changes to proceed. This looks like a great improvement. I've littered the patch with nit-picks, but my main concern is that there aren't any unit tests for the new data structures. I wonder i

[PATCH] D36427: [libcxxabi][demangler] Improve representation of substitutions/templates

2017-08-07 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. Oh, also found a couple of things you should likely split into prep commits to simplify this patch. Comment at: src/cxa_demangle.cpp:1575-1577 -sub_type names; -template_param_type subs; -Vector template_param; - Why not

[PATCH] D34331: func.wrap.func.con: Unset function before destroying anything

2017-08-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In https://reviews.llvm.org/D34331#831686, @arphaman wrote: > Don't you need `// UNSUPPORTED: c++98, c++03` since `std::function` is > supported in C++11 only? The tests in libcxx/test/std/utilities/function.objects/ don't have UNSUPPORTED lines, and I don't see a

[PATCH] D36427: [libcxxabi][demangler] Improve representation of substitutions/templates

2017-08-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. I have a few more nitpicks; LGTM after you fix those. Comment at: src/cxa_demangle.cpp:1621-1631 +// Name stack, this is used by the parser to hold temporary name

[PATCH] D36713: [libc++] Add a persistent way to disable availability

2017-08-14 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D36713 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listi

[PATCH] D92825: [clang][cli] CompilerInvocationTest: join two test fixtures into one

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92825/new/ https://reviews.llvm.org/D92825 _

[PATCH] D92826: [clang][cli] CompilerInvocationTest: rename member variable in fixture

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92826/new/ https://reviews.llvm.org/D92826 _

[PATCH] D92827: [clang][cli] CompilerInvocationTest: split enum test into two

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92827/new/ https://reviews.llvm.org/D92827 _

[PATCH] D92828: [clang][cli] CompilerInvocationTest: remove unnecessary command line arguments

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92828/new/ https://reviews.llvm.org/D92828 _

[PATCH] D92829: [clang][cli] CompilerInvocationTest: check arg parsing does not produce diagnostics

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92829/new/ https://reviews.llvm.org/D92829 _

[PATCH] D92830: [clang][cli] CompilerInvocationTest: join and add test cases

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM; thanks for splitting out the prep commits! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92830/new/ https://reviews.llvm.org/D9283

[PATCH] D92775: [clang][cli] Add flexible TableGen multiclass for boolean options

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM if you undo the change to lose support for AlwaysEmit (happy to consider in a separate patch if it’s the right thing to do though). Comment at: clang/include/cl

[PATCH] D92774: [clang][cli] CompilerInvocationTest: add tests for boolean options

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92774/new/ https://reviews.llvm.org/D92774 _

[PATCH] D91297: Frontend: Take VFS and MainFileBuffer by reference in PrecompiledPreamble::CanReuse, NFC

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. @kadircet, ping, do you still have concerns? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91297/new/ https://reviews.llvm.org/D91297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D91296: Frontend: Clarify logic for using the preamble in ASTUnit::CodeComplete, almost NFC

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG51f3432f4b52: Frontend: Clarify logic for using the preamble in ASTUnit::CodeComplete, almost… (authored by dexonsmith). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D92630: ARCMigrate: Use hash_combine in the DenseMapInfo for EditEntry

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb85c6e5bcd1a: ARCMigrate: Use hash_combine in the DenseMapInfo for EditEntry (authored by dexonsmith). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92630/ne

[PATCH] D91317: Support: Add RedirectingFileSystem::create from simple list of redirections

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added inline comments. Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:735 + /// Redirect each of the remapped files from first to second. + static std::unique_ptr + create(ArrayRef> RemappedFiles, JDevlieghere wrote: > - Do you need to k

[PATCH] D92888: ADT: Allow IntrusiveRefCntPtr construction from std::unique_ptr, NFC

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith created this revision. dexonsmith added reviewers: JDevlieghere, dblaikie. Herald added subscribers: usaxena95, ributzka, kadircet, arphaman. dexonsmith requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-commits. Allow a `std::unique_pt

[PATCH] D91317: Support: Add RedirectingFileSystem::create from simple list of redirections

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added inline comments. Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:735 + /// Redirect each of the remapped files from first to second. + static std::unique_ptr + create(ArrayRef> RemappedFiles, dexonsmith wrote: > JDevlieghere wrote: >

[PATCH] D92627: Basic: Add hashing support for FileEntryRef and DirectoryEntryRef

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added inline comments. Comment at: clang/include/clang/Basic/DirectoryEntry.h:104 + bool isSpecialDenseMapKey() const { +return ME == llvm::DenseMapInfo::getEmptyKey() || + ME == llvm::DenseMapInfo::getTombstoneKey(); jansvoboda11 wrote:

[PATCH] D92888: ADT: Allow IntrusiveRefCntPtr construction from std::unique_ptr, NFC

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. Thanks for the review! Comment at: llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:180 + template + IntrusiveRefCntPtr(std::unique_ptr &&S) : Obj(S.release()) { +retain(); dblaikie wrote: > Pass 'S' by value here - that's the usual

[PATCH] D92888: ADT: Allow IntrusiveRefCntPtr construction from std::unique_ptr, NFC

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5207f19d103d: ADT: Allow IntrusiveRefCntPtr construction from std::unique_ptr, NFC (authored by dexonsmith). Changed prior to commit: https://revi

[PATCH] D91317: Support: Add RedirectingFileSystem::create from simple list of redirections

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG75cd8d756d6e: Support: Add RedirectingFileSystem::create from simple list of redirections (authored by dexonsmith). Changed prior to commit: https://reviews.llvm.org/D91317?vs=304702&id=310398#toc Repo

[PATCH] D92627: Basic: Add hashing support for FileEntryRef and DirectoryEntryRef

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. dexonsmith marked an inline comment as done. Closed by commit rG2878e965af27: Basic: Add hashing support for FileEntryRef and DirectoryEntryRef (authored by dexonsmith). Changed prior to commit: https://reviews.llvm.org/D

[PATCH] D92627: Basic: Add hashing support for FileEntryRef and DirectoryEntryRef

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. Thanks for the reviews! Pushed 2878e965af27ce037378a4f0409e89039108c09f . Comment at: clang/include/clang/Basic/DirectoryEntry.h:104 + bool isSpecialDenseMapKey() const { +ret

[PATCH] D92857: [clang][cli] Don't always emit -f[no-]experimental-new-pass-manager

2020-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. This seems fine to me, I agree this option doesn't need to be present all the time. Thanks for clarifying that `-triple` is still there. @Bigcheese, WDYT? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92857/new/ https:

[PATCH] D92699: Frontend: Migrate to FileEntryRef in VerifyDiagnosticConsumer.cpp, NFC

2020-12-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG82789228c653: Frontend: Migrate to FileEntryRef in VerifyDiagnosticConsumer.cpp, NFC (authored by dexonsmith). Repository: rG LLVM Github Monorepo

[PATCH] D92160: [clang] Fix wrong FDs are used for files with same name in Tooling

2020-12-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. Thanks for your patience; I missed this when I was on holiday, and I'm just noticing it now. Can you tell me more about the scenario this happens? Why is the `FileManager` being reused after the working directory change? Do the `FileEntry`s need to remain valid? I'

[PATCH] D92160: [clang] Fix wrong FDs are used for files with same name in Tooling

2020-12-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D92160#2443405 , @dexonsmith wrote: > - Don't specifically track relative paths / absolute paths. > - Add an API, something like: `dropRelativePaths()`, which drops all > `{File,Directory}Entry{,Ref}` that are somehow based

[PATCH] D92680: Frontend: Migrate to FileEntryRef in CompilerInstance::InitializeSourceManager, NFC

2020-12-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa5c89bb02195: Frontend: Migrate to FileEntryRef in CompilerInstance::InitializeSourceManager… (authored by dexonsmith). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D92967: Tooling: Migrate RewriterTestContext to FileEntryRef, NFC

2020-12-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith created this revision. dexonsmith added reviewers: jansvoboda11, arphaman. Herald added a subscriber: ributzka. dexonsmith requested review of this revision. Herald added a project: clang. Migrate to the `FileEntryRef` overload of `SourceManager::createFileID` (using `FileManager::getOp

[PATCH] D92968: Frontend: Migrate to FileEntryRef in TextDiagnosticTest, NFC

2020-12-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith created this revision. dexonsmith added reviewers: arphaman, jansvoboda11. Herald added a subscriber: ributzka. dexonsmith requested review of this revision. Herald added a project: clang. Migrate over to the `FileEntryRef` overloads of `SourceManager::createFileID` and `overrideFileCon

[PATCH] D92971: clang-import-test: Clean up error output for files that cannot be found

2020-12-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith created this revision. dexonsmith added reviewers: shafik, arphaman, jansvoboda11. Herald added a subscriber: ributzka. dexonsmith requested review of this revision. Herald added a project: clang. Pass on the filesystem error string `FileManager::getFileRef` in `clang-import-test`'s `Pa

[PATCH] D92971: clang-import-test: Clean up error output for files that cannot be found

2020-12-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. If the error output changes seem imprudent, I'm happy to skip those; the only thing I need here is the migration to the other overload of `createFileID` (the "side effect"); could just call `llvm::consumeError` and be done with it. Repository: rG LLVM Github Monor

[PATCH] D92678: ARCMigrate: Migrate ObjCMT.cpp over to FileEntryRef

2020-12-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG898d61b3cff5: ARCMigrate: Migrate ObjCMT.cpp over to FileEntryRef (authored by dexonsmith). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D90889: Remove RemappedFiles param from ASTUnit::LoadFromASTFile, NFC

2020-12-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc3ff9939bf7e: Remove RemappedFiles param from ASTUnit::LoadFromASTFile, NFC (authored by dexonsmith). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D90889?vs=303265&id

[PATCH] D92975: Lex: Migrate HeaderSearch::LoadedModuleMaps to FileEntryRef

2020-12-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith created this revision. dexonsmith added reviewers: arphaman, jansvoboda11. Herald added a subscriber: ributzka. dexonsmith requested review of this revision. Herald added a project: clang. Migrate `HeaderSearch::LoadedModuleMaps` and a number of APIs over to `FileEntryRef`. This should

[PATCH] D92967: Tooling: Migrate some tests to FileEntryRef, NFC

2020-12-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith updated this revision to Diff 310702. dexonsmith retitled this revision from "Tooling: Migrate RewriterTestContext to FileEntryRef, NFC" to "Tooling: Migrate some tests to FileEntryRef, NFC". dexonsmith edited the summary of this revision. dexonsmith added a comment. Found another Tool

[PATCH] D92975: Lex: Migrate HeaderSearch::LoadedModuleMaps to FileEntryRef

2020-12-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith updated this revision to Diff 310713. dexonsmith added a comment. I must have just been building `clangLex` before, since I missed (until now) updating the other API users. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92975/new/ https://reviews.llvm.org/D92975 Files: cla

[PATCH] D92983: SourceManager: Migrate to FileEntryRef in getOrCreateContentCache, NFC

2020-12-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith created this revision. dexonsmith added reviewers: arphaman, jansvoboda11. Herald added a subscriber: ributzka. dexonsmith requested review of this revision. Herald added a project: clang. Change `SourceManager::getOrCreateContentCache` to take a `FileEntryRef` and update call sites (mo

[PATCH] D92984: SourceManager: Change getOrCreateFileID API to take FileEntryRef, NFC

2020-12-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith created this revision. dexonsmith added reviewers: arphaman, jansvoboda11, JDevlieghere. Herald added subscribers: ributzka, jfb. dexonsmith requested review of this revision. Herald added a project: clang. Update `SourceManager::getOrCreateFileID` to take a `FileEntryRef` and migrate a

[PATCH] D92531: Basic: Support named pipes natively in SourceManager

2020-12-10 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith updated this revision to Diff 311004. dexonsmith retitled this revision from "Reapply "Frontend: Sink named pipe logic from CompilerInstance down to FileManager"" to "Basic: Support named pipes natively in SourceManager". dexonsmith edited the summary of this revision. dexonsmith added

[PATCH] D92531: Basic: Support named pipes natively in SourceManager

2020-12-10 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith marked an inline comment as done. dexonsmith added inline comments. Comment at: clang/lib/Basic/SourceManager.cpp:567 + if (IR.ContentsEntry->isNamedPipe()) +(void)IR.getBufferOrNone(Diag, getFileManager(), SourceLocation()); + jansvoboda11 wrote:

[PATCH] D92160: [clang] Fix wrong FDs are used for files with same name in Tooling

2020-12-10 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added subscribers: Bigcheese, jansvoboda11, arphaman. dexonsmith added a comment. In D92160#2444732 , @OikawaKirie wrote: > - In my solution, I use a pretty straightforward approach, which is to have > an individual cache (for `FileEntry` and a

[PATCH] D92160: [clang] Fix wrong FDs are used for files with same name in Tooling

2020-12-10 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D92160#2447586 , @dexonsmith wrote: > /// Current working directory. Grabbed from the (new) VFS whenever it's > /// changed, and updated if the file manager is notified that the VFS's > /// CWD changes. > std::string

[PATCH] D92967: Tooling: Migrate some tests to FileEntryRef, NFC

2020-12-10 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG494aacd72c6a: Tooling: Migrate some tests to FileEntryRef, NFC (authored by dexonsmith). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92967/new/ https://re

[PATCH] D93008: [clang][cli] Do not marshall only CC1Option flags in BoolOption

2020-12-10 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93008/new/ https://reviews.llvm.org/D93008 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D83892: [clang][cli] Port CodeGen option flags to new option parsing system

2020-12-10 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. Thanks for working through this! Updated patch LGTM, with one nit. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:314 FrontendOptions &FrontendOpts = Invoca

[PATCH] D84188: Port FileSystem options to new option parsing system

2020-12-11 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84188/new/ https://reviews.llvm.org/D84188 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D84018: [clang][cli] Port Preprocessor and PreprocessorOutput option flags to new option parsing system

2020-12-11 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84018/new/ https://reviews.llvm.org/D84018

[PATCH] D92160: [clang] Fix wrong FDs are used for files with same name in Tooling

2020-12-11 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D92160#2447861 , @OikawaKirie wrote: > Replies from the original author Hao Zhang > > llvm::ErrorOr > FileManager::getFile(StringRef Filename, bool openFile, bool CacheFailure) { > // Froce using the absolute path to

[PATCH] D93094: [clang][cli] Prevent double denormalization

2020-12-11 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added inline comments. Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:267 - ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fdebug-pass-manager"))); + ASSERT_EQ(count(GeneratedArgs, "-fdebug-pass-manager"), 1); ASSERT_THAT(GeneratedArgs, Not(Contain

[PATCH] D92775: [clang][cli] Add flexible TableGen multiclass for boolean options

2020-12-11 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. (LGTM, in case my conditional approval earlier wasn't clear...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92775/new/ https://reviews.llvm.org/D92775 ___

[PATCH] D92857: [clang][cli] Don't always emit -f[no-]legacy-pass-manager

2020-12-11 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. LGTM too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92857/new/ https://reviews.llvm.org/D92857 ___ cfe-commits mailing list cfe-commits@

[PATCH] D92774: [clang][cli] CompilerInvocationTest: add tests for boolean options

2020-12-11 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92774/new/ https://reviews.llvm.org/D92774

[PATCH] D83979: [clang][cli] Port LangOpts option flags to new option parsing system

2020-12-11 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith requested changes to this revision. dexonsmith added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Driver/Options.td:1292-1303 def fdwarf_exceptions : Flag<["-"], "fdwarf-exceptions">, Group, - Flags<[CC1Option]>, Hel

<    1   2   3   4   5   6   7   8   9   10   >