dexonsmith added inline comments.
================
Comment at: lib/Lex/FilterToIncludes.cpp:628
+ First = Id.Last;
+ auto Kind = llvm::StringSwitch<TokenKind>(Id.Name)
+ .Case("include", pp_include)
----------------
arphaman wrote:
> arphaman wrote:
> > arphaman wrote:
> > > ddunbar wrote:
> > > > What is our feeling w.r.t. _Pragma, which can in theory influence the
> > > > preprocessor. I'm not sure this model can sanely support it?
> > > We need to look into that. In the worst case we can always avoid
> > > minimizing the file if it has a _Pragma anywhere in it.
> > We also have to handle cases like this one:
> >
> > foo.h:
> > ```
> > #define PP _Pragma
> > ```
> >
> > bar.h:
> > ```
> > PP ("foo")
> > ```
> >
> > Unfortunately this can't be handled by just disabling minimization for
> > `foo.h` as PP will be stripped out from `bar.h`. Furthermore, this pattern
> > is quite reasonable, so we should expect it in the code we receive. Right
> > now I can't think of a reasonable solution for this problem.
> >
> > There's also a more "evil" case:
> >
> > ```
> > #define P(A, B) A##B
> >
> > P(_Pra,gma)("foo")
> > ```
> >
> > It would be reasonable to introduce a warning for the use of `_Pragma`
> > token that was created using PP token concatenation and just hope that our
> > users won't use this pattern.
> One possible solution to the first issue is to simply fallback to the regular
> -Eonly invocation if we detect an include of a file that has a macro with a
> `_Pragma` in it. Then we could emit a remark to the user saying that their
> build could be faster if they rewrite their code so that this pattern no
> longer occurs.
Hmm. Our plan for `@import` is to stop supporting code such as:
```
#define IMPORT(M) @import M
IMPORT(LLVMSupport);
```
where the @import is buried. I.e., start erroring out in normal Clang builds.
Perhaps it would be reasonable to similarly disallow `_Pragma` usage that
buries import/include statements. I.e., do not support (even in normal builds)
the following:
```
#define IMPORT(M) _Pragma("clang module import " #M)
IMPORT(LLVMSupport)
```
Possibly, this could be a warning that is error-by-default.
Repository:
rC Clang
https://reviews.llvm.org/D53354
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
