r216620 - Query CompilationDatabase right before running each compilation.

[Alexander Kornienko]

On Fri, Aug 29, 2014 at 6:55 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
> On Thu, Aug 28, 2014 at 8:03 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
>>
>> On Wed, Aug 27, 2014 at 4:28 PM, Alexander Kornienko <alexfh at google.com>
>> wrote:
>>
>>> On Thu, Aug 28, 2014 at 1:03 AM, Sean Silva <chisophugis at gmail.com>
>>> wrote:
>>>
>>>> On Wed, Aug 27, 2014 at 2:36 PM, Alexander Kornienko <alexfh at google.com
>>>> > wrote:
>>>>
>>>>> Author: alexfh
>>>>> Date: Wed Aug 27 16:36:39 2014
>>>>> New Revision: 216620
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=216620&view=rev
>>>>> Log:
>>>>> Query CompilationDatabase right before running each compilation.
>>>>>
>>>>> Summary:
>>>>> Query CompilationDatabase right before running each compilation. This
>>>>> allows
>>>>> supporting compilation databases that change external state required
>>>>> for
>>>>> successful compilation.
>>>>>
>>>>
>>>> Could you give an example of this? It's not clear to me what this means.
>>>>
>>>
>>> This means, that in some implementations the call to
>>> Compilations.getCompileCommands(File) may make changes in the file system
>>> to allow compilation of the File (e.g. generate headers from .td files).
>>> State of the file system required for compiling one file may be
>>> incompatible with the state required for compiling another file, so we
>>> actually need to run the tool on the file right after we call
>>> getCompileCommands for this file.
>>>
>>
>> This is really unusual. I wouldn't expect a call getCompileCommands to be
>> mucking about on a filesystem. Especially since it is marked const. Maybe
>> split out a specific method for preparing the filesystem for compilation of
>> the file? IIRC, in C++11 it's really a very bad idea to have a const method
>> that is not safe to call in parallel (David, do you remember what the rule
>> is for this?)
>>
>
> I don't believe C++11 actually makes this any worse - while C++11 does
> have defined threading semantics, it doesn't actually require that types
> are thread safe. I know Herb or others have certainly talked about
> definitions of "thread compatibility" that are related to const-ness, but I
> don't think any of that is part of the standard. I could be wrong, though.
>
>
>> Also, what is there to stop individual compilations of the same file from
>> having incompatible state?
>>
>
> Yeah, it's certainly worth documenting what the contract is here (& maybe
> considering other APIs).
>
> Probably wouldn't be too bad to make it non-const and/or have a "prep/end"
> function or somesuch (make return a move-only token that represents the
> valid state - when that's destroyed the state is no longer required - but
> perhaps that's just overengineering).
>


We could add pre-post methods or do something else, but it would serve a
purpose of one edge case, which we want to avoid eventually anyway. There
was no particular reason to query compilation database in advance, so this
change add support of this edge case by flipping to an otherwise equal
alternative. I can document this in a comment, but I don't think we need to
invest any more time in developing this workaround further.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140829/91eb6415/attachment.html>