[cfe-commits] r165746 - in /cfe/trunk: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticLexKinds.td include/clang/Lex/MacroInfo.h lib/Lex/MacroInfo.cpp lib/Lex/PPMacroExpansion.cpp lib/Serialization/ASTReader.cpp test/Modules

[Douglas Gregor]

On Oct 12, 2012, at 9:52 AM, Matthieu Monrocq <matthieu.monrocq at gmail.com> wrote:

> 
> 
> On Thu, Oct 11, 2012 at 11:07 PM, Douglas Gregor <dgregor at apple.com> wrote:
> Author: dgregor
> Date: Thu Oct 11 16:07:39 2012
> New Revision: 165746
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=165746&view=rev
> Log:
> Diagnose the expansion of ambiguous macro definitions. This can happen
> only with modules, when two disjoint modules #define the same
> identifier to different token sequences.
> 
> 
> Would it make sense to diagnose an ambiguity even if the macros both expand to the same sequence of tokens ?

I don't think it's practical. Programmers tend to be very cavalier about defining common macros (search your system for all the definitions of NULL, for example), and we're likely to 

> I am concerned with the future actually. If I am linking in two modules and they "work for now" but later on one of them change its macro definition and not the other, I'll get the warning very late.

Changing a macro definition is an API change. I think it's fair to assume that kind of change will have an effect downstream, and fine for it to produce a warning if it introduces a problem.

> (I understand it's somewhat of a subjective opinion, but since there is a precedent with ODR for inline functions...)


The precedent supports the behavior I've implemented. C99 6.10.3p2 allows a macro to be redefined so long as the new definition is identical to the old definition. Thus, it makes sense for macro definitions in two different modules to be permitted so long as those definitions are identical.

OTOH, one cannot define an inline function twice  in the same translation unit, so it should also be an error with modules.

	- Doug
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121012/f8aee7f3/attachment.html>