On Feb 5, 2014, at 10:48 AM, Ben Langmuir <[email protected]> wrote:
> Hi again,
>
> This patch grew a bit to handle a bunch of other non-top-level contexts that
> imports might show up in. Previously annot_module_include signaled an
> end-of-module, which the parser used to stop parsing in a bunch of places,
> and then you would get unfortunate diagnostics like this when the parser
> broke out of whatever context it was in
>
> void foo() {
> #include “bar.h” // error missing }
> } // error extraneous closing brace (})
>
>
> I wasn’t sure how to say ‘must be at top level’ in the diagnostic. I only
> found one existing diagnostic that mentions ‘top level’, but several that
> refer to ‘file scope’, so I went with the latter. I’m happy to change that
> if there is a convention I didn’t pick up on.
I think inside a namespace is also “file scope” so I prefer “top level scope”
here.
Some other nitpicks:
-Watch out for 80 columns violations.
-I’d prefer that we reduce the amount of passing around of the
"llvm::PointerIntPair<Module *, 2, tok::IncludeDirectiveKind> object", e.g.
maybe have a
getModuleImportAnnotationInfo
method in Token which will return a std::pair of (Module *, DirectiveKind) or a
wrapper struct of such info, or return the Module* directly and the
DirectiveKind as an optional out parameter.
>
> Ben
>
> <import.patch>
>
> On Feb 4, 2014, at 8:48 AM, Douglas Gregor <[email protected]> wrote:
>
>>
>> On Feb 4, 2014, at 8:19 AM, Ben Langmuir <[email protected]> wrote:
>>
>>>
>>> On Feb 3, 2014, at 4:51 PM, Douglas Gregor <[email protected]> wrote:
>>>
>>>>
>>>> On Feb 3, 2014, at 2:32 PM, Ben Langmuir <[email protected]> wrote:
>>>>
>>>>> Based on a suggestion from Jordan I’ve dropped the extra note, which will
>>>>> be on the same location anyway, and added that information into the error
>>>>> diagnostic. I would have liked to say "treating #include ...” instead of
>>>>> "treating directive …", but after the preprocessor/lexer this
>>>>> information is lost and I don’t see a nice way to pass it on to the
>>>>> parser.
>>>>
>>>> There’s an ugly way to pass it on to the parser… you have a couple low
>>>> bits in the token’s annotation value to record #include vs. #import vs.
>>>> #include_next.
>>>
>>> To be clear, are you suggesting making Token::PtrData a PointerIntPair?
>>
>> Token::PtrData is an opaque pointer with different meanings for different
>> annotation token kinds. For a module import token, that could be
>> PointerIntPair<Module *, 2, IncludeDirectiveKind>.
>>
>> - Doug
>>
>
> _______________________________________________
> cfe-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
