benlangmuir (Ben Langmuir)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 5 2013, 8:31 AM (250 w, 18 h)

Recent Activity

Tue, Nov 14

benlangmuir requested changes to D39990: Use TempFile in the implementation of LockFileManager.

Spotted one issue, but otherwise seems okay to me. I'd like @bruno to take a look at this, because he's thought about this code much more recently than me.

Tue, Nov 14, 11:18 AM

Aug 30 2017

benlangmuir added a comment to D34512: Add preliminary Cross Translation Unit support library.

In either case, the important scenario I think we should support is choosing at a call site to a C function the most likely definition of the called function, based on number and type of parameters, from multiple possible definitions in other translation units. If the API is rich enough to support this then I think that is a good indication it will be useful for other scenarios as well.

Note that the lookup is already based on USR which is similar to mangled names in a sense that it contains information about the function parameters. So the only way to get multiple candidates from the lookup is having multiple function definitions with the same signature.

Aug 30 2017, 9:53 AM

Aug 16 2017

benlangmuir committed rL311053: [index] Add indexing for unresolved-using declarations.
[index] Add indexing for unresolved-using declarations
Aug 16 2017, 4:14 PM

Jul 21 2017

benlangmuir committed rL308800: [index] Set SymbolSubKind::Accessor[GS]etter on class methods.
[index] Set SymbolSubKind::Accessor[GS]etter on class methods
Jul 21 2017, 4:05 PM

Jul 12 2017

benlangmuir committed rL307855: [index] Don't add relation to a NamedDecl with no name.
[index] Don't add relation to a NamedDecl with no name
Jul 12 2017, 3:05 PM

Jun 21 2017

benlangmuir accepted D34462: [index] "SpecializationOf" relation should be added even to forward declarations of class template specializations.
Jun 21 2017, 10:20 AM

Jun 20 2017

benlangmuir accepted D34392: [index] Nested class declarations should be annotated with the "specializationOf" relation if they pseudo-override a type in the base template.
Jun 20 2017, 10:41 AM

Jun 16 2017

benlangmuir added a comment to D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros.

Thanks, this looks good to me. I'd appreciate if @klimek could take a quick look though.

Jun 16 2017, 2:51 PM
benlangmuir added inline comments to D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros.
Jun 16 2017, 11:23 AM
benlangmuir added a comment to D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros.

It would be nice if the doc comment for the single file parse mode flag was updated to include this new functionality.

Jun 16 2017, 10:34 AM
benlangmuir accepted D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only.

LGTM, thanks!

Jun 16 2017, 9:30 AM

Jun 15 2017

benlangmuir added a comment to D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only.

I agree with not detecting these during PP-only, but there's nothing wrong with <#>. It's either not a placeholder, or it's part of a placeholder like <#>#>, which is a placeholder containing the text ">". Similarly, <## could be the start of an empty placeholder <##>.

Jun 15 2017, 4:21 PM

Jun 14 2017

benlangmuir accepted D33913: [index] Index static_assert declarations.
Jun 14 2017, 3:02 PM
benlangmuir accepted D33920: [index] Record C++17 binding declarations.
Jun 14 2017, 3:01 PM
benlangmuir accepted D34173: [Completion] Code complete the members for a dependent type after a '::' .
Jun 14 2017, 3:01 PM

May 30 2017

benlangmuir committed rL304239: [llvm-config] Fix cflags test looking for "warning".
[llvm-config] Fix cflags test looking for "warning"
May 30 2017, 1:22 PM

May 18 2017

benlangmuir accepted D33324: [index] Avoid infinite recursion when looking up a dependent name.

LGTM. One stylistic comment, but I'll leave that up to you.

May 18 2017, 10:42 AM

May 9 2017

benlangmuir accepted D32972: [index] Index simple dependent declaration references.

A couple of minor comments, but otherwise LGTM.

May 9 2017, 9:29 AM

Apr 20 2017

benlangmuir accepted D32020: [indexer] The relationship between the declarations in template specializations that 'override' declarations in the base template should be recorded.
Apr 20 2017, 8:45 AM

Apr 18 2017

benlangmuir accepted D32081: Add support for editor placeholders to Clang's lexer.
Apr 18 2017, 10:11 AM
benlangmuir added inline comments to D32081: Add support for editor placeholders to Clang's lexer.
Apr 18 2017, 8:48 AM

Apr 14 2017

benlangmuir added a comment to D32081: Add support for editor placeholders to Clang's lexer.

What do you think about a hybrid approach: Don't suppress diagnostics in the placeholder range when we handle the placeholders in parser/sema, but do suppress the range when the placeholder isn't explicitly handled?

Apr 14 2017, 1:40 PM
benlangmuir accepted D32010: [indexer] Record class template specializations using a new 'SpecializationOf' relationship.

LGTM

Apr 14 2017, 11:39 AM
benlangmuir added a comment to D32081: Add support for editor placeholders to Clang's lexer.

Rather than stick ranges into the diagnostic engine, I think it would be cleaner to have the identifier have a method like isEditorPlaceholder() that can be used to avoid situations like this in a principled way, or to customize behaviour for placeholders in the parser, etc. That's how we are handling it in Swift. Using an API on the placeholder is also better for handling errors that could be caused by the placeholder but not have it as the primary location.

Apr 14 2017, 11:27 AM

Mar 17 2017

benlangmuir accepted D28299: Module: use PCMCache to manage memory buffers for pcm files..
Mar 17 2017, 9:58 AM

Feb 10 2017

benlangmuir accepted D28286: [CodeCompletion] Code complete the missing C++11 keywords.

LGTM

Feb 10 2017, 7:24 AM

Jan 20 2017

benlangmuir accepted D28299: Module: use PCMCache to manage memory buffers for pcm files..

Sorry for the delay, I though you were still iterating with @aprantl for some reason. LGTM

Jan 20 2017, 9:49 AM

Jan 10 2017

benlangmuir added inline comments to D28299: Module: use PCMCache to manage memory buffers for pcm files..
Jan 10 2017, 4:11 PM

Jan 9 2017

benlangmuir added a comment to D28415: PCH: fix a regression that reports a module is defined in both pch and pcm.

LGTM

Jan 9 2017, 9:38 AM

Jan 4 2017

benlangmuir added a comment to D28299: Module: use PCMCache to manage memory buffers for pcm files..

Can we test the already-validated diagnostics?

Jan 4 2017, 3:56 PM

Nov 15 2016

benlangmuir added a comment to D25916: Modules: emit an error instead of a random crash (or a misleading error) due to use-after-free..

I'm inclined to suggest we break that behaviour with modules and treat a user module the same no matter where it was imported. @rsmith, what do you think?

Nov 15 2016, 11:24 AM

Nov 14 2016

benlangmuir added a comment to D25916: Modules: emit an error instead of a random crash (or a misleading error) due to use-after-free..

Does it mean that a system module should only import system modules? If a system module is allowed to import non-system modules, for a non-system module, we will validate diagnostic options differently depending on whether a system module or a non-system module imports it. This will cause a non-system module that was validated earlier to be invalidated by a child thread.

Nov 14 2016, 1:26 PM

Nov 8 2016

benlangmuir accepted D25948: [VFS] Replace TimeValue usage with std::chrono.

LGTM, sorry for the delay.

Nov 8 2016, 9:31 AM

Aug 25 2016

benlangmuir added a comment to D23858: Don't diagnose non-modular includes when we are not compiling a module.

LGTM, but @rsmith should glance at this too.

Aug 25 2016, 8:50 AM

Aug 11 2016

benlangmuir added a comment to D23422: [VFS] Add 'ignore-non-existent-contents' field to YAML.

This should have a test for the writer change. Otherwise LGTM.

Aug 11 2016, 2:48 PM

Aug 9 2016

benlangmuir accepted D23125: Modules: add command line option to support loading prebuilt modules on demand, without parsing any module map.

Still missing the change for if (IsPrebuilt) {, but otherwise LGTM.

Aug 9 2016, 4:10 PM
benlangmuir added a comment to D23125: Modules: add command line option to support loading prebuilt modules on demand, without parsing any module map.

Sorry for the delay, I apparently forgot to hit submit. Replied inline.

Aug 9 2016, 9:09 AM

Aug 3 2016

benlangmuir added a comment to D23125: Modules: add command line option to support loading prebuilt modules on demand, without parsing any module map.

How about -fmodules-use-prebuilt-module-cache for the flag name? Saying "prebuilt-modules" is confusing to me, since -fmodule-file can also be used to load a prebuilt module, but doesn't use a cache.

Aug 3 2016, 11:09 AM

Jul 26 2016

benlangmuir accepted D22773: Modules: add command line option fmodules-disable-diagnostic-validation to disable validation of the diagnostic options when loading the module.

LGTM

Jul 26 2016, 9:39 AM

Jul 25 2016

benlangmuir added a comment to D22773: Modules: add command line option fmodules-disable-diagnostic-validation to disable validation of the diagnostic options when loading the module.

the pcm could still be rewritten by a compilation that doesn't use a PCH, and then it would be out of date because of the timestamp instead of the diagnostic options

"a compilation that doesn't use a PCH", is that a different project? And we have two projects building in parallel? Just to make sure I understand.

Jul 25 2016, 4:42 PM
benlangmuir added a comment to D22773: Modules: add command line option fmodules-disable-diagnostic-validation to disable validation of the diagnostic options when loading the module.

We need to add this option to the module hash (see getModuleHash - we already add a bunch of other HSOpts there). Otherwise the pcm could still be rewritten by a compilation that doesn't use a PCH, and then it would be out of date because of the timestamp instead of the diagnostic options.

Jul 25 2016, 2:47 PM

Jul 22 2016

benlangmuir added a comment to D22636: Module: retry building modules that were just compiled by the same instance and are are out of date.

Can you point me to the source codes where we use rename to replace the file? I am curious on how this all works out.

Jul 22 2016, 10:59 AM

Jul 21 2016

benlangmuir added a comment to D22636: Module: retry building modules that were just compiled by the same instance and are are out of date.

B.pcm becomes out of date when we try to load module "A", because another instance overwrites "B.pcm"

Jul 21 2016, 11:36 AM

Jul 14 2016

benlangmuir committed rL275496: Remove the new module cache from the index-module test.
Remove the new module cache from the index-module test
Jul 14 2016, 4:00 PM
benlangmuir committed rL275464: Attempt to workaround Windows bots after my previous commit.
Attempt to workaround Windows bots after my previous commit
Jul 14 2016, 1:16 PM
benlangmuir committed rL275454: [index] Index system ImportDecls even when there is a DeclarationsOnly filter.
[index] Index system ImportDecls even when there is a DeclarationsOnly filter
Jul 14 2016, 11:59 AM

Jun 27 2016

benlangmuir added a comment to D21111: Avoid accessing an invalid PresumedLoc.

LGTM

Jun 27 2016, 12:54 PM

Jun 2 2016

benlangmuir accepted D20942: [LockFileManager] Improve error output by adding error messages.

LGTM. Some minor suggestions.

Jun 2 2016, 9:39 PM

May 23 2016

benlangmuir updated subscribers of D20383: PCH + Module: make sure we write out macros associated with builtin identifiers.

I'd like to see Doug and/or Richard review this. It seems reasonable to me to first blush, but I assume there was a good reason we weren't doing this already...

May 23 2016, 11:17 AM

May 16 2016

benlangmuir accepted D20266: [Modules] Use vfs for (recursive) directory iteration.

LGTM

May 16 2016, 8:44 AM

May 13 2016

benlangmuir accepted D20194: [ModuleMap][CrashReproducer] Collect headers from inner frameworks.

LGTM

May 13 2016, 3:22 PM
benlangmuir added a comment to D20194: [ModuleMap][CrashReproducer] Collect headers from inner frameworks.

Let's move the code looks up the alternate name out of the ModuleMap parser, and into the dependency collector callbacks. This feels like an implementation detail of the dependency collector. We could add a new callback specifically for umbrella headers and handle this inside there.

May 13 2016, 8:59 AM

May 5 2016

benlangmuir added a comment to D17820: Clang Code Completion Filtering .

LGTM with one comment about the doxygen comments, but you should probably wait to hear from @akyrtzi too.

May 5 2016, 8:51 AM

May 3 2016

benlangmuir committed rL268471: Fix CodeCompletion & TypoCorrection when combining a PCH with Modules.
Fix CodeCompletion & TypoCorrection when combining a PCH with Modules
May 3 2016, 5:59 PM

Apr 26 2016

benlangmuir added a comment to D17820: Clang Code Completion Filtering .

Overall approach seems good to me, but this needs tests. Other feedback inline.

Apr 26 2016, 11:28 AM

Apr 12 2016

benlangmuir added inline comments to D18849: [modules] Avoid module relocation error when building modules from VFS.
Apr 12 2016, 12:47 PM
benlangmuir added a comment to D18849: [modules] Avoid module relocation error when building modules from VFS.

From what I understand from http://lists.llvm.org/pipermail/cfe-dev/2014-July/038273.html, changing the behavior of the cached result has side-effects and demands a non trivial change, am I assuming the right thing?

Apr 12 2016, 10:02 AM

Apr 11 2016

benlangmuir added a comment to D18849: [modules] Avoid module relocation error when building modules from VFS.

This doesn't appear to be a safe change. We can't assume that RootName wouldn't ever be legitimately found somewhere in a path (even without a VFS) and changing the path prefix could give a completely different location. I would also be concerned about the performance impact of performing two lookups, since finding a module for a header is not always a cheap operation.

Apr 11 2016, 1:11 PM

Mar 30 2016

benlangmuir accepted D18585: [CrashReproducer] Add a module map callback for added headers.

LGTM

Mar 30 2016, 1:14 PM

Mar 25 2016

benlangmuir committed rL264423: [index] Remove redundancy between symbol kind and language.
[index] Remove redundancy between symbol kind and language
Mar 25 2016, 10:07 AM

Mar 9 2016

benlangmuir committed rL263076: [Modules] Add stdatomic to the list of builtin headers.
[Modules] Add stdatomic to the list of builtin headers
Mar 9 2016, 3:36 PM

Mar 2 2016

benlangmuir added a comment to D17299: [ASTReader] Report error when accessing corrupt record data.

This LGTM, but I would like Richard to take a look too.

Mar 2 2016, 10:55 AM

Feb 25 2016

benlangmuir committed rL261887: Add FieldNames to __NSConstantString_tag.
Add FieldNames to __NSConstantString_tag
Feb 25 2016, 8:41 AM

Feb 19 2016

benlangmuir accepted D17457: [VFS] Add 'overlay-relative' field to YAML files.

LGTM!

Feb 19 2016, 6:14 PM
benlangmuir accepted D17104: [VFS] Drop path traversal assertion.

LGTM if you fix 256->PATH_MAX.

Feb 19 2016, 6:00 PM

Feb 15 2016

benlangmuir added a comment to D17104: [VFS] Drop path traversal assertion.

Okay, let's go with the two-path solution. Thanks!

Feb 15 2016, 4:47 PM

Feb 12 2016

benlangmuir added a comment to D17104: [VFS] Drop path traversal assertion.

Given the two-path solution, another thing we could do in the first path, is to default to the remove_dots() version for the source, i.e. "/install-dir/lib/clang/3.8.0/include" instead of "/install-dir/bin/../lib/clang/3.8.0/include". The path after remove_dots() might be different from what realpath() would return (but this is covered in the 2nd path) but at least we canonicalize for a future virtual path request (which can safely request the VFS based on the returned path from remove_dots()). What do you think about it?

Feb 12 2016, 11:01 AM

Feb 11 2016

benlangmuir added a comment to D17104: [VFS] Drop path traversal assertion.

Now, we try to use the new YAML file + cache from a new clang invocation. When the FileManager requests status in (2) for the path "/install-dir/bin/../lib/clang/3.8.0/include/altivec.h" neither "remove_dots()" nor "realpath()" would be able to resolve it to "/private/tmp/a/b/lib/clang/3.8.0/include/altivec.h" and will fallback to search the real filesystem instead, if this is a different machine, it will fail. Since we are looking at a virtual path in (2) it could makes sense to use remove_dots() but it won't make sense to use "realpath()".

How do you propose we fix that?

Feb 11 2016, 1:28 PM
benlangmuir committed rL260563: [Modules] Early-exit if ReadOptionsBlock fails to avoid crashing.
[Modules] Early-exit if ReadOptionsBlock fails to avoid crashing
Feb 11 2016, 10:58 AM
benlangmuir committed rL260543: [Modules] Don't infinite recurse on implicit import of circular modules in….
[Modules] Don't infinite recurse on implicit import of circular modules in…
Feb 11 2016, 9:09 AM

Feb 10 2016

benlangmuir added a comment to D17104: [VFS] Drop path traversal assertion.

I don't think this is the right approach. If we don't canonicalize the source path then:

  • looking up the path *without* the .. won't work, which means anything that looks up a realpath will fail
  • directory iteration won't combine entries from foo/bar/.. and foo/
Feb 10 2016, 3:14 PM
benlangmuir added a comment to D17104: [VFS] Drop path traversal assertion.

Please clarify what you mean here:

The rationale is that if source and destination paths in the YAML file contain ".." this is enough
for the file manager to retrieve the right file, meaning that it doesn't matter how we write it
since the FileManager is capable of transforming it in real/feasible paths.

Feb 10 2016, 2:53 PM

Feb 4 2016

benlangmuir committed rL259859: Don't synthesize an ImportDecl for a module named in -fmodule-implementation-of.
Don't synthesize an ImportDecl for a module named in -fmodule-implementation-of
Feb 4 2016, 5:14 PM

Feb 3 2016

benlangmuir committed rL259734: Fix predefine for __NSConstantString struct type.
Fix predefine for __NSConstantString struct type
Feb 3 2016, 4:59 PM

Feb 2 2016

benlangmuir committed rL259624: Make CF constant string decl visible to name lookup to fix module errors.
Make CF constant string decl visible to name lookup to fix module errors
Feb 2 2016, 7:30 PM

Dec 11 2015

benlangmuir committed rL255377: Reapply "[Modules] Fix regression when an elaborated-type-specifier mentions….
Reapply "[Modules] Fix regression when an elaborated-type-specifier mentions…
Dec 11 2015, 2:08 PM

Dec 10 2015

benlangmuir committed rL255324: Revert "[Modules] Fix regression when an elaborated-type-specifier mentions a….
Revert "[Modules] Fix regression when an elaborated-type-specifier mentions a…
Dec 10 2015, 5:47 PM
benlangmuir committed rL255312: [VFS] Fix status() of opened redirected file.
[VFS] Fix status() of opened redirected file
Dec 10 2015, 3:44 PM
benlangmuir committed rL255267: [Modules] Fix regression when an elaborated-type-specifier mentions a hidden tag.
[Modules] Fix regression when an elaborated-type-specifier mentions a hidden tag
Dec 10 2015, 9:32 AM

Nov 13 2015

benlangmuir committed rL253123: [modules] Allow "redefinition" of typedef of anon tag from unimported submodule.
[modules] Allow "redefinition" of typedef of anon tag from unimported submodule
Nov 13 2015, 7:28 PM

Oct 28 2015

benlangmuir committed rL251565: Fix missing builtin identifier infos with PCH+modules.
Fix missing builtin identifier infos with PCH+modules
Oct 28 2015, 3:28 PM

Oct 21 2015

benlangmuir committed rL250963: Fix use-after-free in ModuleManager.
Fix use-after-free in ModuleManager
Oct 21 2015, 4:15 PM

Aug 13 2015

benlangmuir committed rL244917: Attempt to fix build after r244912.
Attempt to fix build after r244912
Aug 13 2015, 10:30 AM
benlangmuir closed D11403: [Modules] Add Darwin-specific compatibility module map parsing hacks.

Any regression to code completion was below the noise level in my testing.

Aug 13 2015, 10:20 AM
benlangmuir committed rL244912: [Modules] Add Darwin-specific compatibility module map parsing hacks.
[Modules] Add Darwin-specific compatibility module map parsing hacks
Aug 13 2015, 10:14 AM

Aug 12 2015

benlangmuir updated the diff for D11403: [Modules] Add Darwin-specific compatibility module map parsing hacks.

Changes per review. I still need to check performance of code-completing import statements to make sure the extra directory iteration isn't a big regression.

Aug 12 2015, 8:57 AM

Aug 6 2015

benlangmuir closed D11824: Make target feature 'arm' cover both 32 and 64 bit architecutres.

r244306

Aug 6 2015, 8:06 PM
benlangmuir committed rL244306: Make 'arm' cover both 32 and 64 bit architecutres.
Make 'arm' cover both 32 and 64 bit architecutres
Aug 6 2015, 7:00 PM
benlangmuir added inline comments to D11824: Make target feature 'arm' cover both 32 and 64 bit architecutres.
Aug 6 2015, 4:49 PM
benlangmuir retitled D11824: Make target feature 'arm' cover both 32 and 64 bit architecutres from to Make target feature 'arm' cover both 32 and 64 bit architecutres.
Aug 6 2015, 4:10 PM

Aug 5 2015

benlangmuir added inline comments to D11403: [Modules] Add Darwin-specific compatibility module map parsing hacks.
Aug 5 2015, 3:01 PM
benlangmuir added inline comments to D11403: [Modules] Add Darwin-specific compatibility module map parsing hacks.
Aug 5 2015, 2:36 PM
benlangmuir updated the diff for D11403: [Modules] Add Darwin-specific compatibility module map parsing hacks.

Sorry for the slow response, I somehow missed your original comment.

Aug 5 2015, 8:43 AM

Jul 21 2015

benlangmuir added a comment to D10423: [modules] PR20507: Avoid silent textual inclusion..

Darwin module map hacks: http://reviews.llvm.org/D11403
We also need to fix clang's _Builting_Intrinsics.arm module. I posted a thread about this to cfe-dev for feedback: http://lists.cs.uiuc.edu/pipermail/cfe-dev/2015-July/044225.html

Jul 21 2015, 8:26 PM
benlangmuir retitled D11403: [Modules] Add Darwin-specific compatibility module map parsing hacks from to [Modules] Add Darwin-specific compatibility module map parsing hacks.
Jul 21 2015, 8:21 PM

Jul 20 2015

benlangmuir added a comment to D10423: [modules] PR20507: Avoid silent textual inclusion..

Since Richard accepted this, I'll pipe up that I have a patch adding the hacks needed to make this work on Darwin. I need to add some tests and then I'll put it up for review - hopefully tomorrow.

Jul 20 2015, 5:28 PM

Jul 13 2015

benlangmuir added a comment to D10423: [modules] PR20507: Avoid silent textual inclusion..

Something weird seems to have been going on with Phab emails and I missed your patch for the header/requires ordering issue. Anyway, I fixed it in r242055 with a similar approach.

Jul 13 2015, 1:04 PM
benlangmuir committed rL242055: [Modules] Allow missing header before a missing requirement.
[Modules] Allow missing header before a missing requirement
Jul 13 2015, 12:49 PM

Jul 8 2015

benlangmuir added a comment to D10423: [modules] PR20507: Avoid silent textual inclusion..
  • Added better source location.
Jul 8 2015, 12:58 PM
benlangmuir added a comment to D10423: [modules] PR20507: Avoid silent textual inclusion..

I agree that exclude makes more sense for requires excluded as a "supported" feature, but if this is another egregious hack for Darwin, I don't think it matters which we choose.

Jul 8 2015, 11:45 AM