r265766 - [modules] Add a comment to explain why -E leaves some #includes in the preprocessed output.

[Richard Smith via cfe-commits]

On Mon, Apr 18, 2016 at 11:49 AM, Vassil Vassilev <v.g.vassilev at gmail.com>
wrote:

> On 18/04/16 20:32, Richard Smith wrote:
>
> On Mon, Apr 18, 2016 at 6:28 AM, Vassil Vassilev <
> <v.g.vassilev at gmail.com>v.g.vassilev at gmail.com> wrote:
>
>> On 08/04/16 03:24, Richard Smith via cfe-commits wrote:
>>
>>> Author: rsmith
>>> Date: Thu Apr  7 20:23:59 2016
>>> New Revision: 265766
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=265766&view=rev
>>> Log:
>>> [modules] Add a comment to explain why -E leaves some #includes in the
>>> preprocessed output.
>>>
>>> Modified:
>>>      cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp
>>>      cfe/trunk/test/Modules/preprocess.cpp
>>>
>>> Modified: cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp?rev=265766&r1=265765&r2=265766&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp (original)
>>> +++ cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp Thu Apr  7
>>> 20:23:59 2016
>>> @@ -336,7 +336,9 @@ void PrintPPOutputPPCallbacks::Inclusion
>>>         OS << "#include "
>>>            << (IsAngled ? '<' : '"')
>>>            << FileName
>>> -         << (IsAngled ? '>' : '"');
>>> +         << (IsAngled ? '>' : '"')
>>> +         << " /* clang -E: implicit import for module "
>>> +         << Imported->getFullModuleName() << " */";
>>>
>> It seems that in some cases the FileName needs to be tweaked to be able
>> to compile the output back. For instance:
>> clang -I folder/ file.cxx
>> cat file.cxx
>> #include "subfolder/A.h"
>>
>> cat folder/subfolder/A.h
>> #include "B.h"
>>
>> B.h resides in folder/subfolder/ and FileName argument would be B.h
>> causing the printer to generate #include "B.h" /* clang -E: implicit import
>> for... */ which cannot be compiled back.
>
>
> Ugh, yeah, that makes sense.
>
> It seems superficially that what we should do for a file found relative to
> the current file (or in MSVC mode, for a file found relative to a
> possibly-indirect includer of the current file) is to prepend the path from
> that file's search path to the current file. That is, if we find
> "foo/bar.h" in search path "includes/x", and we find "baz/quux.h" relative
> to bar.h, we should produce the path "foo/baz/quux.h" (to be found relative
> to "includes/x"). However, that won't work if there is a prior include path
> that also contains a "foo/baz/quux.h", so we would need to also include the
> search path in the include path. And *that* won't work if . is not a search
> path and one of the search paths is a relative path.
>
> :( at first looked like an easy, nice solution, but apparently "twin
> peaks" killer phase is true again :)
>
>
> I wonder whether the problem is really that we're handling #include search
> paths incorrectly in the presence of #line markers.
>
> I am not sure I understand this. Could you give an example?
>

Suppose we have:

foo/bar.h:
#include "baz.h"

foo/baz.h:
// ...

foo/module.modulemap
textual header "bar.h"
module Baz { header "baz.h" }

foo.cc:
#include "foo/bar.h"

We currently produce a preprocessed source file like this:

# 1 "foo/bar.h"
#include "baz.h"

This is, in some sense, "correct" -- the #include line from "foo/bar.h" did
say "baz.h". But we'll look for "baz.h" relative to the current
preprocessed source file, not relative to "foo/bar.h".

We could change the preprocessor so that in the above case it searches
relative to "foo/" instead, but that will break a collection of other use
cases (and is different from GCC's behavior, and breaks the common flow of
deleting the #line markers when reducing a test case, and so on). So
instead, we could provide a modified syntax for the above that requests
that the #include of "baz.h" be found relative to "foo/bar.h". We would
only need to do this in the narrow case that (a) the include names a module
header, and (b) that module header was found relative to the including
file, and (c) the including file was itself included into the main source
file.

Another alternative would be to provide a module import keyword that works
across languages. Given that the C++ Modules TS is making good progress,
perhaps we can just ignore this problem for now (this change is at least
not a regression) and solve it by generating an import once we implement
the real syntax.


> Perhaps for the relative path search we should somehow instruct clang to
> look for files relative to the presumed file rather than the physical file?
>
> Could we detect that we are compiling a preprocessed file? I guess there
> are traces, eg. include stack push/pop values...
>
> That would match our intent for these files. However, this doesn't match
> the GCC behavior, so we probably can't do it by default. =/
>
>
>
>
>>       }
>>>       // Since we want a newline after the @import, but not a #<line>,
>>> start a new
>>>       // line immediately.
>>>
>>> Modified: cfe/trunk/test/Modules/preprocess.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/preprocess.cpp?rev=265766&r1=265765&r2=265766&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/test/Modules/preprocess.cpp (original)
>>> +++ cfe/trunk/test/Modules/preprocess.cpp Thu Apr  7 20:23:59 2016
>>> @@ -1,6 +1,6 @@
>>>   // RUN: rm -rf %t
>>>   // RUN: %clang_cc1 -fmodules -fimplicit-module-maps
>>> -fmodules-cache-path=%t -I %S/Inputs -x c++ -E %s | \
>>> -// RUN:   FileCheck -strict-whitespace %s --check-prefix=CHECK
>>> --check-prefix=CXX
>>> +// RUN:   FileCheck -strict-whitespace %s --check-prefix=CHECK
>>> --check-prefix=CXX --check-prefix=CXX-DASHE
>>>   // RUN: %clang_cc1 -fmodules -fimplicit-module-maps
>>> -fmodules-cache-path=%t -I %S/Inputs -x objective-c -E %s | \
>>>   // RUN:   FileCheck -strict-whitespace %s --check-prefix=CHECK
>>> --check-prefix=OBJC
>>>   // RUN: %clang_cc1 -fmodules -fimplicit-module-maps
>>> -fmodules-cache-path=%t -I %S/Inputs -x c++ -E -frewrite-includes %s | \
>>> @@ -14,7 +14,9 @@ foo bar baz
>>>   // The weird {{ }} here is to prevent the -frewrite-includes test from
>>> matching its own CHECK lines.
>>>     // CXX: #include{{ }}"dummy.h"
>>> +// CXX-DASHE-SAME: /* clang -E: implicit import for module dummy */
>>>   // CXX: #include{{ }}"dummy.h"
>>> +// CXX-DASHE-SAME: /* clang -E: implicit import for module dummy */
>>>   // CXX: foo bar baz
>>>     // OBJC: @import{{ }}dummy; /* clang
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160418/a2088efa/attachment.html>